diff --git a/TODO/02-reduce-spec-doubles.md b/TODO/02-reduce-spec-doubles.md index ffa2abaf..e26304b3 100644 --- a/TODO/02-reduce-spec-doubles.md +++ b/TODO/02-reduce-spec-doubles.md @@ -1,45 +1,163 @@ -# 18 — Reduce spec doubles (69 sites) +# 18 — Reduce spec doubles -**Priority:** Medium (spec quality) -**Files:** Multiple spec files, worst offenders: -- `spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb` (~20) -- `spec/uniword/accessibility/rules_integration_spec.rb` (~10) -- `spec/uniword/accessibility/accessibility_checker_spec.rb` (~7) -- `spec/uniword/math_equation_spec.rb` (~6) +**Status:** Items 1-3 done. Item 4 outstanding. Count went 59 → 30 (29 +addressed). Both contract defects are fixed: `ImageAltTextRule` now reads +`wp:docPr/@descr`, and `WordCss` now speaks `Wordprocessingml::Style`'s +real API (`id`, `font_family`, `alignment.value`, plus a new `Style#italic`). -## Problem +What remains, by file: + +| file | sites | why | +| --- | --- | --- | +| `spec/uniword/math_equation_spec.rb` | 10 (7 `double`, 3 `class_double`) | Plurimath — out of scope | +| `spec/uniword/math/plurimath_adapter_spec.rb` | 7 | Plurimath — out of scope | +| `spec/uniword/accessibility/rules_integration_spec.rb` | 5 | table/heading rules — item 4 | +| `spec/uniword/accessibility/accessibility_checker_spec.rb` | 2 (1 `double`, 1 `instance_double`) | item 4 | +| 6 single-site files | 6 | item 4, fold in opportunistically | + +**Priority:** Medium for the spec cleanup; the two contract repairs below are +correctness fixes. +**Files:** see the work items below. + +## Status of the original note -Project rule: never use `double()` in specs. Use real model instances or -`Struct.new(...).new(...)` for plain data. +Stale. It claimed 69 sites. Actual today, counting all RSpec double +constructs: **59** — 55 plain `double(...)`, 3 `class_double`, 1 +`instance_double`. Its named worst offenders are no longer the worst. +Distribution of plain doubles: -69 `double()` callsites across spec/. Worst offenders use doubles to -mock out complex model behavior, which means the tests verify that -methods are called rather than that the actual behavior is correct. +| file | sites | +| --- | --- | +| `spec/uniword/mhtml/word_css_spec.rb` | 14 | +| `spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb` | 12 | +| `spec/uniword/math_equation_spec.rb` | 7 | +| `spec/uniword/math/plurimath_adapter_spec.rb` | 7 | +| `spec/uniword/accessibility/rules_integration_spec.rb` | 7 | +| `spec/uniword/accessibility/accessibility_checker_spec.rb` | 2 (+1 `instance_double`) | +| 6 other files | 1 each | -## Fix +The original fix section also said to use `Struct` for data-only +doubles. `CLAUDE.md` says never use `Struct` or `OpenStruct`. Follow +CLAUDE.md; this note predates the rule. Two `Struct.new` sites already +exist in `spec/` — either violations to clean up or a rule needing a +written exception. Do not add a third. -For each spec: -1. Identify what the double is mocking -2. Replace with a real model instance constructed with the required - attributes -3. If the model is hard to set up, build a small test factory -4. Verify the test still asserts the same observable behavior +## Problem + +The doubles are not lazy tests. Two of them are hiding broken production +code, and that has to be fixed before the cleanup can even run. -For data-only doubles (no behavior), use `Struct`: -```ruby -# Before -let(:doc) { double("Doc", paragraphs: [...]) } +### `ImageAltTextRule` crashes on any real document with an image -# After -DocumentStub = Struct.new(:paragraphs) -let(:doc) { DocumentStub.new([...]) } ``` +document.images => [Uniword::Wordprocessingml::Drawing] +Drawing responds to alt_text? => false +rule.check(doc) => NoMethodError: + undefined method 'alt_text' for + an instance of Wordprocessingml::Drawing +``` + +`DocumentRoot#images` (`document_root.rb:299`) is documented +`@return [Array]`. `ImageAltTextRule#check` calls +`image.alt_text` (`image_alt_text_rule.rb:21`). `alt_text` exists only on +`Uniword::Image` (`image.rb:28`), a different flat model that `#images` +never returns. The 12 doubles inject `alt_text` and paper over the crash. + +### `WordCss` expects an API the real objects do not have + +The doubles invent `style_id`, `font` and `italic`. Real +`Wordprocessingml::Style` exposes `id` (`style.rb:130`) and +`font_family` (`style.rb:200`), and has no `italic` reader. +`Mhtml::StylesConfiguration#styles` is declared +`attribute :styles, :hash` (`styles_configuration.rb:15`), not an +enumerable of style objects. + +So "replace the double with a real instance" cannot be done for these +two without fixing production first. + +## 1. Repair the accessibility contract (prerequisite) + +Decide what `ImageAltTextRule` operates on. Most likely real OOXML +drawings, reading the description off `wp:docPr/@descr`, not a phantom +`alt_text` on `Drawing`. Then replace the 12 doubles in +`image_alt_text_rule_spec.rb` and the dependent doubles in +`rules_integration_spec.rb` and `accessibility_checker_spec.rb`. + +Regression test must be a parsed document containing a real drawing, and +must fail before the fix. + +## 2. Repair the WordCss contract (prerequisite) -For doubles that mock method calls, refactor the test to assert on -observable output (return values, side effects) rather than on -method-call counts. +Decide whether `WordCss` consumes WordprocessingML styles or MHTML style +hashes, then make the code and the type agree. Replace the 7 style-side +doubles (5 `Style`, 2 `StylesConfiguration`). + +The namespace is **not** ambiguous, despite two classes sharing the name: +`word_css.rb:36` does `styles_config.styles.map` and then calls style-object +methods, while `Mhtml::StylesConfiguration#styles` is a plain `:hash` of CSS +properties. So `WordCss` consumes the **WordprocessingML** configuration. State +that as the contract. + +The real work is adapting `WordCss` to `Wordprocessingml::Style`'s actual API: +it exposes `id` (`style.rb:130`) and `font_family` (`style.rb:200`), has a +wrapper-backed `alignment`, and has no `italic` reader at all — whereas the +doubles invent `style_id`, `font` and `italic`. + +## 3. WordCss numbering doubles + +The other 7 doubles in that file are 5 `NumberingInstance` and 2 +`NumberingConfiguration`. + +**This is independent of item 2 and can be done on its own.** `word_css.rb:50` calls +`numbering_config.instances`, and only `Wordprocessingml::NumberingConfiguration` +declares `instances` (`numbering_configuration.rb:16`); the MHTML one does not. +So the namespace is settled by the call itself, and a real +`NumberingConfiguration` plus `NumberingInstance` drops straight in. + +Two earlier revisions got this wrong in both directions — first calling it +unblocked without checking, then calling it blocked on a namespace ambiguity +that does not exist. The call site resolves it. + +## 4. The remaining doubles + +Whatever is left after items 1-3, largest file first, one file per PR. +For each double: name the real class, construct it the way production +does, keep the assertion on observable behavior rather than call counts. + +If a model turns out to be awkward to construct, that awkwardness is a +finding about the model's API. Record it. Do not paper over it with +another double. ## Verification -`grep -rn "double(" spec/uniword/ | wc -l` should trend toward 0. -Tests pass. +- Per file: `bundle exec rspec ` green **and proven able to fail**. + Break the behavior under test once and watch it go red. Given what items + 1 and 2 uncovered, a green double-free spec that cannot fail is the + main risk here. +- The count drops from 59 by the number actually addressed. Count all three + constructs (`double`, `class_double`, `instance_double`), not just plain + `double(` — an earlier revision of this note undercounted by missing + `class_double`. **Do not state a target of zero.** List what remains + explicitly. +- `bundle exec rubocop ` + +## Out of scope + +- `plurimath_adapter_spec.rb` and the Plurimath parts of + `math_equation_spec.rb` (14 sites). These stand in for an external + gem, so replacing them is a dependency-contract question, not spec + hygiene. Needs `/dependency-contract-check` against Plurimath first. + Should become its own TODO. +- The 6 single-site files. Fold in only if the file is already open for + another reason. + +## Expected outcome, stated honestly + +This will not reduce the count much on its own. Its real output is the two +contract defects in items 1 and 2. + +Severity differs between them. `ImageAltTextRule` is reachable and crashes on a +real document. `WordCss.generate_style_css` / `generate_list_css` have **no +caller under `lib/`** — only their specs call them — so they are broken public +helpers rather than a demonstrated downstream failure. Fix both, but do not +describe WordCss as a live production bug. diff --git a/lib/uniword/accessibility/rules/image_alt_text_rule.rb b/lib/uniword/accessibility/rules/image_alt_text_rule.rb index 6e8e4dc7..73dcb52b 100644 --- a/lib/uniword/accessibility/rules/image_alt_text_rule.rb +++ b/lib/uniword/accessibility/rules/image_alt_text_rule.rb @@ -18,14 +18,15 @@ def check(document) violations = [] document.images.each_with_index do |image, index| - # Check for alt text existence - if image.alt_text.nil? || image.alt_text.strip.empty? + alt_text = image.alt_text + + # Drawing#alt_text already treats a blank descr as absent. + if alt_text.nil? violations << create_violation( message: "Image #{index + 1} missing alternative text", element: image, severity: @config[:severity] || :error, - suggestion: @config[:suggestion] || - "Add descriptive alternative text using image.alt_text = '...'", + suggestion: missing_alt_text_suggestion(image), ) next # Skip quality checks if no alt text end @@ -33,7 +34,7 @@ def check(document) # Check alt text quality if enabled next unless @config[:check_quality] - violations.concat(check_alt_text_quality(image, index)) + violations.concat(check_alt_text_quality(image, alt_text, index)) end violations @@ -41,18 +42,39 @@ def check(document) private + # Word's older Alt Text dialog had both a Title and a Description + # field, and templates from that era put the description in Title. + # Title is a caption, not a text alternative, so the image still + # counts as undescribed — but say where the text already is. + # + # @param drawing [Wordprocessingml::Drawing] Drawing being reported + # @return [String] Suggestion text + def missing_alt_text_suggestion(drawing) + title = drawing.alt_title + if title + return "Move the drawing's docPr title (#{title.inspect}) into " \ + "its descr attribute; title is a caption, not alt text" + end + + @config[:suggestion] || + "Add descriptive alternative text via the drawing's " \ + "docPr descr attribute" + end + # Check quality of alt text # - # @param image [Image] Image to check + # @param image [Wordprocessingml::Drawing] Drawing to check + # @param alt_text [String] Alternative text read from the drawing # @param index [Integer] Image index # @return [Array] Quality violations - def check_alt_text_quality(image, index) + def check_alt_text_quality(image, alt_text, index) violations = [] # Check minimum length - if @config[:min_length] && image.alt_text.length < @config[:min_length] + if @config[:min_length] && alt_text.length < @config[:min_length] violations << create_violation( - message: "Image #{index + 1} has insufficient alt text (too short: #{image.alt_text.length} chars)", + message: "Image #{index + 1} has insufficient alt text " \ + "(too short: #{alt_text.length} chars)", element: image, severity: :warning, suggestion: "Alternative text should describe the image content meaningfully (min #{@config[:min_length]} chars)", @@ -60,9 +82,10 @@ def check_alt_text_quality(image, index) end # Check maximum length - if @config[:max_length] && image.alt_text.length > @config[:max_length] + if @config[:max_length] && alt_text.length > @config[:max_length] violations << create_violation( - message: "Image #{index + 1} has excessive alt text (too long: #{image.alt_text.length} chars)", + message: "Image #{index + 1} has excessive alt text " \ + "(too long: #{alt_text.length} chars)", element: image, severity: :warning, suggestion: "Keep alternative text concise (max #{@config[:max_length]} chars)", @@ -71,12 +94,12 @@ def check_alt_text_quality(image, index) # Check for unhelpful generic text unhelpful = %w[image picture photo img graphic icon] - alt_lower = image.alt_text.downcase + alt_lower = alt_text.downcase if unhelpful.any? do |word| alt_lower == word || alt_lower.start_with?("#{word} of") end violations << create_violation( - message: "Image #{index + 1} has generic alt text: '#{image.alt_text}'", + message: "Image #{index + 1} has generic alt text: '#{alt_text}'", element: image, severity: :warning, suggestion: "Describe what the image shows, not that it's an image", diff --git a/lib/uniword/builder/image_builder.rb b/lib/uniword/builder/image_builder.rb index 46e0f2bd..1258ab5e 100644 --- a/lib/uniword/builder/image_builder.rb +++ b/lib/uniword/builder/image_builder.rb @@ -147,6 +147,7 @@ def self.create_drawing(document, path, width: nil, height: nil, inline.doc_properties = WpDrawing::DocProperties.new( id: deterministic_id("inline", path), name: File.basename(path, ".*"), + descr: alt_text, ) inline.graphic = build_graphic(r_id, w, h) @@ -227,6 +228,7 @@ def self.create_floating(document, path, width: nil, height: nil, alt_text: nil, anchor.doc_properties = WpDrawing::DocProperties.new( id: deterministic_id("anchor", path), name: File.basename(path, ".*"), + descr: alt_text, ) anchor.graphic = build_graphic(r_id, w, h) diff --git a/lib/uniword/builder/sdt_builder.rb b/lib/uniword/builder/sdt_builder.rb index 9cb9521e..f4cd6e37 100644 --- a/lib/uniword/builder/sdt_builder.rb +++ b/lib/uniword/builder/sdt_builder.rb @@ -63,19 +63,27 @@ def alias(value) # Set the lock / content cannot be edited # + # Passing false writes an explicit rather + # than leaving the flag out, so the intent survives a round trip. + # # @param value [Boolean] Lock content (default true) # @return [self] - def lock(_value = true) - properties.temporary = Wordprocessingml::StructuredDocumentTag::Temporary.new + def lock(value = true) + properties.temporary = + Wordprocessingml::StructuredDocumentTag::Temporary.new(value: value) self end # Set placeholder text showing the placeholder header # + # Passing false writes an explicit , + # the same way #lock does. + # # @param value [Boolean] Show placeholder (default true) # @return [self] - def showing_placeholder(_value = true) - properties.showing_placeholder_header = Wordprocessingml::StructuredDocumentTag::ShowingPlaceholderHeader.new + def showing_placeholder(value = true) + klass = Wordprocessingml::StructuredDocumentTag::ShowingPlaceholderHeader + properties.showing_placeholder_header = klass.new(value: value) self end diff --git a/lib/uniword/mhtml/word_css.rb b/lib/uniword/mhtml/word_css.rb index 70d1845d..88501afe 100644 --- a/lib/uniword/mhtml/word_css.rb +++ b/lib/uniword/mhtml/word_css.rb @@ -119,29 +119,37 @@ def self.build_section_div_rule(section_name) # Build a CSS rule for a style. # - # @param style [Style] The style + # @param style [Wordprocessingml::Style] The style # @return [String, nil] The CSS rule or nil def self.build_style_rule(style) return nil unless style + # w:styleId is optional in the schema, and without it there is no + # class selector to hang the rule on. + style_id = style.id + return nil if style_id.nil? || style_id.empty? + properties = [] # Font properties - properties << "font-family: '#{style.font}'" if style.font + font_family = style.font_family + properties << "font-family: '#{font_family}'" if font_family if style.font_size - properties << "font-size: #{CssNumberFormatter.format(style.font_size, 'pt', - precision: 1)}" + # w:sz is in half-points, so it needs the font-size formatter. + formatted_size = CssNumberFormatter + .format_font_size(style.font_size, precision: 1) + properties << "font-size: #{formatted_size}" end properties << "font-weight: bold" if style.bold properties << "font-style: italic" if style.italic - # Paragraph properties - properties << "text-align: #{style.alignment}" if style.alignment + # Paragraph properties, whose alignment is a w:jc wrapper element + alignment = style.alignment&.value + properties << "text-align: #{alignment}" if alignment return nil if properties.empty? - selector = ".#{style.style_id}" - "#{selector} {\n #{properties.join(";\n ")};\n}" + ".#{style_id} {\n #{properties.join(";\n ")};\n}" end # Build a CSS rule for list numbering. diff --git a/lib/uniword/ooxml/types/ooxml_boolean.rb b/lib/uniword/ooxml/types/ooxml_boolean.rb index 9d67dd09..aa2bf1fc 100644 --- a/lib/uniword/ooxml/types/ooxml_boolean.rb +++ b/lib/uniword/ooxml/types/ooxml_boolean.rb @@ -11,8 +11,9 @@ module Types # # Parsing: "1"/"true"/"on" -> true, "0"/"false"/"off"/nil -> false # Serialization: true -> "1", false -> "0" - # Anything else raises Lutaml::Model::Type::InvalidValueError - # instead of passing through unchanged. + # Serializing anything else raises + # Lutaml::Model::Type::InvalidValueError, because a value the program + # set is a bug rather than a document we were handed. class OoxmlBoolean < Lutaml::Model::Type::Boolean # Accepted ST_OnOff spellings for Boolean true TRUE_VALUES = [true, 1, "1", "true", "on"].freeze @@ -23,14 +24,25 @@ class OoxmlBoolean < Lutaml::Model::Type::Boolean # All accepted ST_OnOff spellings ON_OFF_VALUES = (TRUE_VALUES + FALSE_VALUES).freeze + # The one ST_OnOff reading, shared by these attribute types and by + # Properties::BooleanElement. + # + # A reader must not raise on a malformed document. One bad token in + # styles.xml used to kill the whole parse, and a token outside the + # vocabulary is not an off token, so it reads as on — the same way an + # unrecognised w:val does. + # + # @param value [Object] Raw ST_OnOff token + # @return [Boolean] true when the toggle is on + def self.on?(value) + !FALSE_VALUES.include?(value) + end + def self.cast(value, _options = {}) return value if Lutaml::Model::Utils.uninitialized?(value) - return true if TRUE_VALUES.include?(value) - return false if FALSE_VALUES.include?(value) || value.nil? + return false if value.nil? - raise Lutaml::Model::Type::InvalidValueError.new( - value, ON_OFF_VALUES - ) + on?(value) end def self.serialize(value) diff --git a/lib/uniword/ooxml/types/ooxml_boolean_optional.rb b/lib/uniword/ooxml/types/ooxml_boolean_optional.rb index 55e22e91..afb41696 100644 --- a/lib/uniword/ooxml/types/ooxml_boolean_optional.rb +++ b/lib/uniword/ooxml/types/ooxml_boolean_optional.rb @@ -10,9 +10,9 @@ module Types # Key behavior: # - cast(nil) -> nil (doesn't convert to false like OoxmlBoolean) # - cast("1"/"true"/"on") -> true, cast("0"/"false"/"off") -> false - # - cast of any other value raises - # Lutaml::Model::Type::InvalidValueError instead of passing - # through unchanged + # - cast of any other token -> true, the same reading + # Properties::BooleanElement gives an unrecognised w:val. A reader + # must not raise on a malformed document. # - to_xml(true) -> "1" # - to_xml(false) -> "0" (explicit false in original) # - to_xml(nil) -> nil (attribute absent, omit from output) @@ -26,12 +26,8 @@ class OoxmlBooleanOptional < Lutaml::Model::Type::Boolean def self.cast(value, _options = {}) return value if Lutaml::Model::Utils.uninitialized?(value) return nil if value.nil? - return true if OoxmlBoolean::TRUE_VALUES.include?(value) - return false if OoxmlBoolean::FALSE_VALUES.include?(value) - raise Lutaml::Model::Type::InvalidValueError.new( - value, OoxmlBoolean::ON_OFF_VALUES - ) + OoxmlBoolean.on?(value) end def self.serialize(value) diff --git a/lib/uniword/properties/boolean_element_factory.rb b/lib/uniword/properties/boolean_element_factory.rb index cab222cb..6007cf4d 100644 --- a/lib/uniword/properties/boolean_element_factory.rb +++ b/lib/uniword/properties/boolean_element_factory.rb @@ -4,12 +4,57 @@ module Uniword module Properties - # OOXML boolean element mixin for value/val getter logic only. + # Shared reading and writing behaviour for OOXML ST_OnOff elements. + # # The val= setter must be defined AFTER attribute :val to override - # the generated setter. + # the generated setter, so includers pull in BooleanValSetter separately. module BooleanElement + # Move a `value:` key onto `val:`. + # + # lutaml-model drops constructor keys it does not know, so + # `Bold.new(value: "0")` used to build an ON toggle and throw the + # argument away. An explicit `val:` still wins. + # + # @param attrs [Hash, Object] Constructor arguments + # @return [Hash, Object] Arguments with `value` renamed to `val` + def self.alias_value_key(attrs) + return attrs unless attrs.is_a?(Hash) + + key = [:value, "value"].find { |k| attrs.key?(k) } + return attrs if key.nil? + + attrs = attrs.dup + aliased = attrs.delete(key) + attrs[:val] = aliased unless attrs.key?(:val) || attrs.key?("val") + attrs + end + + def initialize(attrs = {}) + super(BooleanElement.alias_value_key(attrs)) + end + + # ST_OnOff (ECMA-376 §17.17.4) spells off as "0", "false" or "off". + # An absent w:val means the toggle is on. Unknown tokens read as on: + # a reader must not raise on a malformed document. + # + # @return [Boolean] true when the toggle is on + def on? + Ooxml::Types::OoxmlBoolean.on?(val) + end + + # One reading for every consumer. + # + # This used to be `val != "false"`, which read "0" and "off" as on. + # Word shows both as off. + # + # @return [Boolean] true when the toggle is on def value - val != "false" + on? + end + + # @param new_value [Object] Any ST_OnOff spelling, or a Ruby boolean + def value=(new_value) + self.val = new_value end end diff --git a/lib/uniword/properties/boolean_formatting.rb b/lib/uniword/properties/boolean_formatting.rb index 624d1090..b96ada42 100644 --- a/lib/uniword/properties/boolean_formatting.rb +++ b/lib/uniword/properties/boolean_formatting.rb @@ -18,5 +18,9 @@ module Properties BooleanElementFactory.define("qFormat", "QuickFormat") BooleanElementFactory.define("keepNext", "KeepNext") BooleanElementFactory.define("keepLines", "KeepLines") + + # Paragraph-level boolean elements + BooleanElementFactory.define("suppressLineNumbers", "SuppressLineNumbers") + BooleanElementFactory.define("bidi", "Bidi") end end diff --git a/lib/uniword/properties/outline.rb b/lib/uniword/properties/outline.rb index bb3a07cc..bc055acb 100644 --- a/lib/uniword/properties/outline.rb +++ b/lib/uniword/properties/outline.rb @@ -9,25 +9,16 @@ module Properties # Represents or # Used in run properties (w:rPr) for outline text effect class Outline < Lutaml::Model::Serializable - attribute :val, :string + include BooleanElement + + attribute :val, :string, default: nil + include BooleanValSetter xml do element "outline" namespace Uniword::Ooxml::Namespaces::WordProcessingML map_attribute "val", to: :val, render_nil: false, render_default: false end - - # Handle boolean-like values for val attribute - # nil = true (element present without val means true) - # 'false' = false - def initialize(attrs = {}) - if [true, "true"].include?(attrs[:val]) - attrs[:val] = nil # true = no val attribute - elsif [false, "false"].include?(attrs[:val]) - attrs[:val] = "false" - end - super - end end end end diff --git a/lib/uniword/quality/rules/image_alt_text_rule.rb b/lib/uniword/quality/rules/image_alt_text_rule.rb index 1de308f5..4960b848 100644 --- a/lib/uniword/quality/rules/image_alt_text_rule.rb +++ b/lib/uniword/quality/rules/image_alt_text_rule.rb @@ -33,55 +33,34 @@ def check(document) return violations unless @require_alt_text - image_count = 0 - document.paragraphs.each_with_index do |para, para_index| - # Images are in runs as drawings - para.runs.each do |run| - run.drawings.each do |drawing| - image_count += 1 + # Same drawings the accessibility rule and the renderer see, table + # cells included, read through the same Drawing#alt_text. + document.images.each_with_index do |drawing, index| + image_count = index + 1 + alt_text = drawing.alt_text - alt_text = extract_alt_text(drawing) - - if alt_text.nil? || alt_text.empty? - violations << create_violation( - severity: :error, - message: "Image #{image_count} is missing alt text. " \ - "Alt text is required for accessibility.", - location: "Paragraph #{para_index + 1}, Image #{image_count}", - element: drawing, - ) - elsif alt_text.length < @min_length - violations << create_violation( - severity: :warning, - message: "Image #{image_count} has alt text that is too short " \ - "(#{alt_text.length} characters, minimum: #{@min_length}). " \ - "Provide more descriptive alt text.", - location: "Paragraph #{para_index + 1}, Image #{image_count}", - element: drawing, - ) - end - end + if alt_text.nil? + violations << create_violation( + severity: :error, + message: "Image #{image_count} is missing alt text. " \ + "Alt text is required for accessibility.", + location: "Image #{image_count}", + element: drawing, + ) + elsif alt_text.length < @min_length + violations << create_violation( + severity: :warning, + message: "Image #{image_count} has alt text that is too short " \ + "(#{alt_text.length} characters, minimum: #{@min_length}). " \ + "Provide more descriptive alt text.", + location: "Image #{image_count}", + element: drawing, + ) end end violations end - - private - - # Extract alt text from image (Drawing element) - # - # In OOXML, alt text is stored in: - # - drawing.inline.doc_properties.descr (for inline images) - # - drawing.anchor.doc_properties.descr (for anchored images) - # - # @param drawing [Drawing] The drawing element to extract alt text from - # @return [String, nil] Alt text or nil if not present - def extract_alt_text(drawing) - # Try inline first, then anchor - doc_props = drawing&.inline&.doc_properties || drawing&.anchor&.doc_properties - doc_props&.descr - end end end end diff --git a/lib/uniword/transformation/mhtml_element_renderer.rb b/lib/uniword/transformation/mhtml_element_renderer.rb index 76ccb2f3..3b65d587 100644 --- a/lib/uniword/transformation/mhtml_element_renderer.rb +++ b/lib/uniword/transformation/mhtml_element_renderer.rb @@ -212,7 +212,13 @@ def drawing_to_html(drawing) style_attrs << "height:#{height_px}px" if height_px style = style_attrs.empty? ? "" : " style='#{style_attrs.join(';')}'" - %() + # No alt attribute at all when the drawing carries no description. + # An empty alt says "decorative", which is a claim about the image + # we have no grounds to make. + alt_text = drawing.alt_text + alt = alt_text ? %( alt="#{escape_xml(alt_text)}") : "" + + %() end # Resolve image target path from image_parts @@ -637,9 +643,9 @@ def build_sdt_attrs(props) attrs << %(w:id="#{props.id.value}") if props.id&.value - attrs << 'w:showingPlcHdr="t"' if props.showing_placeholder_header + attrs << 'w:showingPlcHdr="t"' if props.showing_placeholder_header&.on? - attrs << 'w:temporary="t"' if props.temporary + attrs << 'w:temporary="t"' if props.temporary&.on? if props.placeholder&.doc_part doc_part = props.placeholder.doc_part diff --git a/lib/uniword/transformation/ooxml_to_html_converter.rb b/lib/uniword/transformation/ooxml_to_html_converter.rb index 3df6fbef..12817eef 100644 --- a/lib/uniword/transformation/ooxml_to_html_converter.rb +++ b/lib/uniword/transformation/ooxml_to_html_converter.rb @@ -60,9 +60,14 @@ def self.run_to_html(run) props = run.properties return text unless props - # Apply inline formatting - text = "#{text}" if props.bold - text = "#{text}" if props.italic + # Apply inline formatting. + # + # w:b and w:i are ST_OnOff toggles, so they are read through + # BooleanElement#on? and an explicit off ("0"/"false"/"off") stays + # off. The reads below are value elements, not toggles: w:u carries + # an ST_Underline style, the rest carry plain values. + text = "#{text}" if props.bold&.on? + text = "#{text}" if props.italic&.on? text = "#{text}" if props.underline&.value text = "#{text}" if props.color&.value text = "#{text}" if props.size&.value diff --git a/lib/uniword/wordprocessingml.rb b/lib/uniword/wordprocessingml.rb index 301cefa3..17d462cb 100644 --- a/lib/uniword/wordprocessingml.rb +++ b/lib/uniword/wordprocessingml.rb @@ -232,6 +232,9 @@ module Wordprocessingml autoload :HideMark, "uniword/wordprocessingml/hide_mark" autoload :TextDirection, "uniword/wordprocessingml/text_direction" + # Shared YAML serialization helper mixed into the models below + autoload :YamlWriter, "uniword/wordprocessingml/yaml_writer" + # Properties classes (consolidated from Ooxml::WordProcessingML) autoload :ParagraphProperties, "uniword/wordprocessingml/paragraph_properties" diff --git a/lib/uniword/wordprocessingml/document_root.rb b/lib/uniword/wordprocessingml/document_root.rb index 0231b72e..58e97b2d 100644 --- a/lib/uniword/wordprocessingml/document_root.rb +++ b/lib/uniword/wordprocessingml/document_root.rb @@ -292,16 +292,31 @@ def tables body&.tables || [] end + # Every paragraph in the document, including the ones inside table + # cells and inside tables nested in those cells. + # + # Top-level paragraphs come first, then table paragraphs: w:p and w:tbl + # are separate collections on the model, so their interleaving is not + # recoverable here. + # + # @return [Array] All paragraphs, top-level ones first + def all_paragraphs + return [] unless body + + (body.paragraphs || []) + + (body.tables || []).flat_map { |table| table_paragraphs(table) } + end + # Get all drawings (image references) from the document. - # Walks all paragraphs and collects Drawing elements from runs. + # Walks every paragraph, table cells included, and collects Drawing + # elements from runs. A drawing in a table cell is still an image the + # renderer emits, so the accessibility and quality rules must see it. # # @return [Array] All drawing elements in document def images - return [] unless body&.paragraphs - - body.paragraphs.flat_map do |para| - (para.runs || []).flat_map(&:drawings) - end.compact + all_paragraphs + .flat_map { |para| (para.runs || []).flat_map(&:drawings) } + .compact end # @return [Hash] Document statistics (paragraphs, tables, images) @@ -369,6 +384,25 @@ def to_html_document private + # Paragraphs held by a table, walking nested tables as well. + # + # @param table [Table] Table to walk + # @return [Array] Paragraphs in that table + def table_paragraphs(table) + cells = (table.rows || []).flat_map { |row| row.cells || [] } + cells.flat_map { |cell| cell_paragraphs(cell) } + end + + # Paragraphs held by one table cell, including its nested tables. + # + # @param cell [TableCell] Cell to walk + # @return [Array] Paragraphs in that cell + def cell_paragraphs(cell) + nested = (cell.tables || []) + .flat_map { |table| table_paragraphs(table) } + (cell.paragraphs || []) + nested + end + # Run model-level validation rules against this document. # # @return [Array] issues found diff --git a/lib/uniword/wordprocessingml/drawing.rb b/lib/uniword/wordprocessingml/drawing.rb index 45d95b44..cae779f9 100644 --- a/lib/uniword/wordprocessingml/drawing.rb +++ b/lib/uniword/wordprocessingml/drawing.rb @@ -19,6 +19,57 @@ class Drawing < Lutaml::Model::Serializable map_element "inline", to: :inline, render_nil: false map_element "anchor", to: :anchor, render_nil: false end + + # Alternative text for the drawing. + # + # Alt text lives in the descr attribute of wp:docPr. ECMA-376 names + # descr the description of the object; title is the object's caption, + # which Word's modern Alt Text pane does not write and screen readers + # do not announce. Word documents in the ISO corpus carry their alt + # text in descr with no title at all, so title is deliberately not a + # fallback here — see #alt_title. + # + # CT_Drawing is a choice over wp:inline and wp:anchor, so a drawing may + # carry either or both. Take the first frame supplying a non-blank + # description. Surrounding whitespace is not description, so the value + # is stripped and a blank one counts as absent. + # + # @return [String, nil] Alternative text, or nil when absent + def alt_text + first_non_blank(:descr) + end + + # Set the alternative text on whichever frame this drawing carries. + # A drawing with neither frame has nowhere to put it. + # + # @param text [String, nil] Alternative text + def alt_text=(text) + frame = inline || anchor + unless frame.nil? + frame.doc_properties ||= WpDrawing::DocProperties.new + frame.doc_properties.descr = text + end + end + + # The drawing's title (Word's legacy "Title" field), which is a caption + # rather than a text alternative. Reported so a rule can tell an author + # that their description is sitting in the wrong field. + # + # @return [String, nil] Title, or nil when absent + def alt_title + first_non_blank(:title) + end + + private + + # @param attribute [Symbol] docPr attribute to read + # @return [String, nil] First non-blank value across the frames + def first_non_blank(attribute) + [inline, anchor] + .filter_map { |frame| frame&.doc_properties&.public_send(attribute) } + .map(&:strip) + .find { |text| !text.empty? } + end end end end diff --git a/lib/uniword/wordprocessingml/paragraph_properties.rb b/lib/uniword/wordprocessingml/paragraph_properties.rb index b9fc49ff..ee043014 100644 --- a/lib/uniword/wordprocessingml/paragraph_properties.rb +++ b/lib/uniword/wordprocessingml/paragraph_properties.rb @@ -9,6 +9,8 @@ module Wordprocessingml # Represents w:pPr element containing paragraph-level formatting. # Used in StyleSets and document paragraph elements. class ParagraphProperties < Lutaml::Model::Serializable + include YamlWriter + # Pattern 0: ATTRIBUTES FIRST, then XML mappings # Simple element attributes (OOXML w:val attributes stored in @@ -64,10 +66,15 @@ class ParagraphProperties < Lutaml::Model::Serializable # Spacing options attribute :contextual_spacing, Properties::ContextualSpacing - attribute :suppress_line_numbers, :boolean, default: -> { false } + + # w:suppressLineNumbers and w:bidi are ST_OnOff elements, not plain + # booleans. Declared as :boolean they read "" for every spelling, so an + # explicitly-off flag was indistinguishable from an on one, and a + # parsed element was dropped on the way back out. + attribute :suppress_line_numbers_wrapper, Properties::SuppressLineNumbers # Bidirectional text - attribute :bidirectional, :boolean, default: -> { false } + attribute :bidirectional_wrapper, Properties::Bidi # East Asian typography attribute :auto_space_de, Properties::AutoSpaceDE @@ -104,11 +111,15 @@ class ParagraphProperties < Lutaml::Model::Serializable to: :yaml_page_break_before_to } map "outline_level", with: { from: :yaml_outline_level_from, to: :yaml_outline_level_to } - map "suppress_line_numbers", to: :suppress_line_numbers + map "suppress_line_numbers", + with: { from: :yaml_suppress_line_numbers_from, + to: :yaml_suppress_line_numbers_to } map "contextual_spacing", with: { from: :yaml_contextual_spacing_from, to: :yaml_contextual_spacing_to } - map "bidirectional", to: :bidirectional + map "bidirectional", + with: { from: :yaml_bidirectional_from, + to: :yaml_bidirectional_to } map "indent_left", to: :indent_left map "indent_right", to: :indent_right map "indent_first_line", to: :indent_first_line @@ -117,7 +128,9 @@ class ParagraphProperties < Lutaml::Model::Serializable end # YAML transform methods (instance methods called by lutaml-model's - # `with:` transform mechanism) + # `with:` transform mechanism). Every writer assigns through + # YamlWriter#yaml_put; see that module for why. + def yaml_style_from(instance, value) if value instance.style = [ @@ -126,40 +139,42 @@ def yaml_style_from(instance, value) end end - def yaml_style_to(instance, _doc) - Array(instance.style).first&.value + def yaml_style_to(instance, doc) + yaml_put(doc, "style", Array(instance.style).first&.value) end def yaml_alignment_from(instance, value) instance.alignment = Properties::Alignment.new(value: value) if value end - def yaml_alignment_to(instance, _doc) - instance.alignment&.value + def yaml_alignment_to(instance, doc) + yaml_put(doc, "alignment", instance.alignment&.value) end def yaml_keep_next_from(instance, value) instance.keep_next_wrapper = Properties::KeepNext.new(value: value) unless value.nil? end - def yaml_keep_next_to(instance, _doc) - instance.keep_next_wrapper&.value + # Toggles read through BooleanElement#on?, the same reading the XML and + # predicate consumers get, so w:val="0" and w:val="off" stay off. + def yaml_keep_next_to(instance, doc) + yaml_put(doc, "keep_next", instance.keep_next_wrapper&.on?) end def yaml_keep_lines_from(instance, value) instance.keep_lines_wrapper = Properties::KeepLines.new(value: value) unless value.nil? end - def yaml_keep_lines_to(instance, _doc) - instance.keep_lines_wrapper&.value + def yaml_keep_lines_to(instance, doc) + yaml_put(doc, "keep_lines", instance.keep_lines_wrapper&.on?) end def yaml_outline_level_from(instance, value) instance.outline_level = Properties::OutlineLevel.new(value: value.to_i) if value end - def yaml_outline_level_to(instance, _doc) - instance.outline_level&.value + def yaml_outline_level_to(instance, doc) + yaml_put(doc, "outline_level", instance.outline_level&.value) end def yaml_contextual_spacing_from(instance, value) @@ -168,8 +183,8 @@ def yaml_contextual_spacing_from(instance, value) instance.contextual_spacing = Properties::ContextualSpacing.new(value: value) end - def yaml_contextual_spacing_to(instance, _doc) - instance.contextual_spacing&.value + def yaml_contextual_spacing_to(instance, doc) + yaml_put(doc, "contextual_spacing", instance.contextual_spacing&.on?) end def yaml_page_break_before_from(instance, value) @@ -178,8 +193,8 @@ def yaml_page_break_before_from(instance, value) instance.page_break_before_wrapper = Properties::PageBreakBefore.new(value: value) end - def yaml_page_break_before_to(instance, _doc) - instance.page_break_before_wrapper&.value + def yaml_page_break_before_to(instance, doc) + yaml_put(doc, "page_break_before", instance.page_break_before_wrapper&.on?) end def yaml_widow_control_from(instance, value) @@ -188,8 +203,44 @@ def yaml_widow_control_from(instance, value) instance.widow_control_wrapper = Properties::WidowControl.new(value: value) end - def yaml_widow_control_to(instance, _doc) - instance.widow_control_wrapper&.value + def yaml_widow_control_to(instance, doc) + yaml_put(doc, "widow_control", instance.widow_control_wrapper&.on?) + end + + def yaml_suppress_line_numbers_from(instance, value) + return if value.nil? + + instance.suppress_line_numbers_wrapper = + Properties::SuppressLineNumbers.new(value: value) + end + + def yaml_suppress_line_numbers_to(instance, doc) + yaml_put(doc, "suppress_line_numbers", + instance.suppress_line_numbers_wrapper&.on?) + end + + def yaml_bidirectional_from(instance, value) + return if value.nil? + + instance.bidirectional_wrapper = Properties::Bidi.new(value: value) + end + + def yaml_bidirectional_to(instance, doc) + yaml_put(doc, "bidirectional", instance.bidirectional_wrapper&.on?) + end + + # Is line numbering suppressed for this paragraph? + # + # @return [Boolean] false when w:suppressLineNumbers is absent or off + def suppress_line_numbers + suppress_line_numbers_wrapper&.on? || false + end + + # Does this paragraph run right-to-left? + # + # @return [Boolean] false when w:bidi is absent or off + def bidirectional + bidirectional_wrapper&.on? || false end # XML mappings come AFTER attributes @@ -214,8 +265,9 @@ def yaml_widow_control_to(instance, _doc) # Numbering properties (wrapped in w:numPr) map_element "numPr", to: :numbering_properties, render_nil: false - # Suppress line numbers (only render if true) - map_element "suppressLineNumbers", to: :suppress_line_numbers, render_nil: false, + # Suppress line numbers (only render if present) + map_element "suppressLineNumbers", to: :suppress_line_numbers_wrapper, + render_nil: false, render_default: false # Borders (complex object) @@ -233,8 +285,8 @@ def yaml_widow_control_to(instance, _doc) map_element "autoSpaceDN", to: :auto_space_dn, render_nil: false, render_default: false - # Bidirectional (only render if true) - map_element "bidi", to: :bidirectional, render_nil: false, + # Bidirectional (only render if present) + map_element "bidi", to: :bidirectional_wrapper, render_nil: false, render_default: false # Right indent adjustment @@ -271,6 +323,9 @@ def initialize(attrs = {}) keep_lines_val = attrs.key?(:keep_lines) ? attrs.delete(:keep_lines) : nil page_break_before_val = attrs.key?(:page_break_before) ? attrs.delete(:page_break_before) : nil widow_control_val = attrs.key?(:widow_control) ? attrs.delete(:widow_control) : nil + suppress_line_numbers_val = + attrs.key?(:suppress_line_numbers) ? attrs.delete(:suppress_line_numbers) : nil + bidirectional_val = attrs.key?(:bidirectional) ? attrs.delete(:bidirectional) : nil style_val = attrs.key?(:style) ? attrs.delete(:style) : nil super @@ -290,6 +345,14 @@ def initialize(attrs = {}) self.widow_control_wrapper = Properties::WidowControl.new(value: widow_control_val) end + unless suppress_line_numbers_val.nil? + self.suppress_line_numbers_wrapper = + Properties::SuppressLineNumbers.new(value: suppress_line_numbers_val) + end + unless bidirectional_val.nil? + self.bidirectional_wrapper = + Properties::Bidi.new(value: bidirectional_val) + end self.style = style_val if style_val # Convert flat attributes to wrapper objects (Pattern 0: after super) diff --git a/lib/uniword/wordprocessingml/run_properties/conversion.rb b/lib/uniword/wordprocessingml/run_properties/conversion.rb index ab372b8c..aee5ca60 100644 --- a/lib/uniword/wordprocessingml/run_properties/conversion.rb +++ b/lib/uniword/wordprocessingml/run_properties/conversion.rb @@ -9,6 +9,25 @@ class RunProperties < Lutaml::Model::Serializable # (e.g., RunProperties.new(bold: true)) and the wrapper # objects (e.g., Properties::Bold) required by lutaml-model. module Conversion + # Every rPr attribute whose value is an ST_OnOff toggle element. + BOOLEAN_WRAPPERS = { + bold: Properties::Bold, + bold_cs: Properties::BoldCs, + italic: Properties::Italic, + italic_cs: Properties::ItalicCs, + strike: Properties::Strike, + double_strike: Properties::DoubleStrike, + small_caps: Properties::SmallCaps, + caps: Properties::Caps, + hidden: Properties::Vanish, + no_proof: Properties::NoProof, + web_hidden: Properties::WebHidden, + shadow: Properties::Shadow, + emboss: Properties::Emboss, + imprint: Properties::Imprint, + outline: Properties::Outline, + }.freeze + def initialize(attrs = {}) # Extract flat convenience keys before super (lutaml-model ignores # unknown keys). These are converted to proper wrapper objects below. @@ -96,16 +115,18 @@ def apply_shading_overrides(fill_val, type_val) shading.pattern = type_val if type_val end + # Wrap every ST_OnOff toggle handed in as a primitive. + # + # `false` is a value, not an absence: testing truthiness here left a + # raw Ruby false on the attribute and to_xml then died on it. def convert_boolean_attrs! - @bold = Properties::Bold.new(value: @bold) if @bold && !@bold.is_a?(Properties::Bold) - @bold_cs = Properties::BoldCs.new(value: @bold_cs) if @bold_cs && !@bold_cs.is_a?(Properties::BoldCs) - @italic = Properties::Italic.new(value: @italic) if @italic && !@italic.is_a?(Properties::Italic) - @italic_cs = Properties::ItalicCs.new(value: @italic_cs) if @italic_cs && !@italic_cs.is_a?(Properties::ItalicCs) - @strike = Properties::Strike.new(value: @strike) if @strike && !@strike.is_a?(Properties::Strike) - @double_strike = Properties::DoubleStrike.new(value: @double_strike) if @double_strike && !@double_strike.is_a?(Properties::DoubleStrike) - @small_caps = Properties::SmallCaps.new(value: @small_caps) if @small_caps && !@small_caps.is_a?(Properties::SmallCaps) - @caps = Properties::Caps.new(value: @caps) if @caps && !@caps.is_a?(Properties::Caps) - @hidden = Properties::Vanish.new(value: @hidden) if @hidden && !@hidden.is_a?(Properties::Vanish) + BOOLEAN_WRAPPERS.each do |name, klass| + current = instance_variable_get(:"@#{name}") + next if current.nil? || current.is_a?(klass) || + Lutaml::Model::Utils.uninitialized?(current) + + instance_variable_set(:"@#{name}", klass.new(val: current)) + end end def convert_style_attr! diff --git a/lib/uniword/wordprocessingml/run_properties/predicates.rb b/lib/uniword/wordprocessingml/run_properties/predicates.rb index aa19bb4e..24d82963 100644 --- a/lib/uniword/wordprocessingml/run_properties/predicates.rb +++ b/lib/uniword/wordprocessingml/run_properties/predicates.rb @@ -5,30 +5,19 @@ module Wordprocessingml class RunProperties < Lutaml::Model::Serializable # Boolean predicate methods for RunProperties. # - # Unwraps boolean property objects and returns true/false. + # Every predicate reads through BooleanElement#on?, so all of them + # agree on what "0", "off" and an absent w:val mean. module Predicates def bold? - val = bold - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + toggle_on?(bold) end def italic? - val = italic - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + toggle_on?(italic) end def strike? - val = strike - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + toggle_on?(strike) end def all_caps @@ -36,59 +25,45 @@ def all_caps end def caps? - val = caps - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + toggle_on?(caps) end def small_caps? - val = small_caps - return false if val.nil? - - val = val.value if val.is_a?(Properties::SmallCaps) - val == true + toggle_on?(small_caps) end def shadow? - val = shadow - return false if val.nil? - - val = val.value if val.is_a?(Properties::Shadow) - val == true + toggle_on?(shadow) end def imprint? - val = imprint - return false if val.nil? - - val = val.value if val.is_a?(Properties::Imprint) - val == true + toggle_on?(imprint) end def emboss? - val = emboss - return false if val.nil? - - val = val.value if val.is_a?(Properties::Emboss) - val == true + toggle_on?(emboss) end def hidden? - val = hidden - return false if val.nil? - - val = val.value if val.is_a?(Properties::Vanish) - val == true + toggle_on?(hidden) end def outline? - val = outline - return false if val.nil? + toggle_on?(outline) + end + + private + + # Toggles arrive as BooleanElement wrappers when parsed and as plain + # booleans when a caller assigns one directly. + # + # @param toggle [Object, nil] Wrapper, boolean, or nil when absent + # @return [Boolean] true when the toggle is on + def toggle_on?(toggle) + return false if toggle.nil? + return toggle.on? if toggle.is_a?(Uniword::Properties::BooleanElement) - val = val.val if val.is_a?(Properties::Outline) - val != "false" + toggle == true end end end diff --git a/lib/uniword/wordprocessingml/run_properties/yaml_transforms.rb b/lib/uniword/wordprocessingml/run_properties/yaml_transforms.rb index 3dd8fd6d..eb9c6cbc 100644 --- a/lib/uniword/wordprocessingml/run_properties/yaml_transforms.rb +++ b/lib/uniword/wordprocessingml/run_properties/yaml_transforms.rb @@ -8,63 +8,73 @@ class RunProperties < Lutaml::Model::Serializable # Handles bidirectional conversion between flat YAML keys # (bold, italic, size, etc.) and the wrapper objects used # internally by the OOXML model. + # + # Every writer assigns through YamlWriter#yaml_put; see that module + # for why. module YamlTransforms + include YamlWriter + # --- Boolean transforms --- + # + # Toggles read through BooleanElement#on?, the same reading the XML + # and predicate consumers get. def yaml_bold_from(instance, value) instance.bold = Properties::Bold.new(value: value) unless value.nil? end - def yaml_bold_to(_instance, _doc) - bold&.value + def yaml_bold_to(instance, doc) + yaml_put(doc, "bold", instance.bold&.on?) end def yaml_italic_from(instance, value) instance.italic = Properties::Italic.new(value: value) unless value.nil? end - def yaml_italic_to(_instance, _doc) - italic&.value + def yaml_italic_to(instance, doc) + yaml_put(doc, "italic", instance.italic&.on?) end def yaml_strike_from(instance, value) instance.strike = Properties::Strike.new(value: value) unless value.nil? end - def yaml_strike_to(_instance, _doc) - strike&.value + def yaml_strike_to(instance, doc) + yaml_put(doc, "strike", instance.strike&.on?) end def yaml_double_strike_from(instance, value) instance.double_strike = Properties::DoubleStrike.new(value: value) unless value.nil? end - def yaml_double_strike_to(_instance, _doc) - double_strike&.value + def yaml_double_strike_to(instance, doc) + yaml_put(doc, "double_strike", instance.double_strike&.on?) end def yaml_small_caps_from(instance, value) instance.small_caps = Properties::SmallCaps.new(value: value) unless value.nil? end - def yaml_small_caps_to(_instance, _doc) - small_caps&.value + def yaml_small_caps_to(instance, doc) + yaml_put(doc, "small_caps", instance.small_caps&.on?) end def yaml_caps_from(instance, value) instance.caps = Properties::Caps.new(value: value) unless value.nil? end - def yaml_caps_to(_instance, _doc) - caps&.value + # "caps" and "all_caps" are two spellings of one property, so both + # rules write the canonical key rather than emitting it twice. + def yaml_caps_to(instance, doc) + yaml_put(doc, "caps", instance.caps&.on?) end def yaml_hidden_from(instance, value) instance.hidden = Properties::Vanish.new(value: value) unless value.nil? end - def yaml_hidden_to(_instance, _doc) - hidden&.value + def yaml_hidden_to(instance, doc) + yaml_put(doc, "hidden", instance.hidden&.on?) end # --- Numeric transforms --- @@ -73,16 +83,16 @@ def yaml_size_from(instance, value) instance.size = Properties::FontSize.new(value: value.to_i) if value end - def yaml_size_to(_instance, _doc) - size&.value + def yaml_size_to(instance, doc) + yaml_put(doc, "size", instance.size&.value) end def yaml_character_spacing_from(instance, value) instance.character_spacing = Properties::CharacterSpacing.new(value: value.to_i) if value end - def yaml_character_spacing_to(_instance, _doc) - character_spacing&.value + def yaml_character_spacing_to(instance, doc) + yaml_put(doc, "character_spacing", instance.character_spacing&.value) end # --- String transforms --- @@ -91,24 +101,24 @@ def yaml_underline_from(instance, value) instance.underline = Properties::Underline.new(value: value) if value end - def yaml_underline_to(_instance, _doc) - underline&.value + def yaml_underline_to(instance, doc) + yaml_put(doc, "underline", instance.underline&.value) end def yaml_color_from(instance, value) instance.color = Properties::ColorValue.new(value: value) if value end - def yaml_color_to(_instance, _doc) - color&.value + def yaml_color_to(instance, doc) + yaml_put(doc, "color", instance.color&.value) end def yaml_highlight_from(instance, value) instance.highlight = Properties::Highlight.new(value: value) if value end - def yaml_highlight_to(_instance, _doc) - highlight&.value + def yaml_highlight_to(instance, doc) + yaml_put(doc, "highlight", instance.highlight&.value) end # --- Font transforms --- @@ -118,8 +128,10 @@ def yaml_font_from(instance, value) instance.fonts.ascii = value if value end - def yaml_font_to(_instance, _doc) - fonts&.ascii + # "font" and "font_ascii" both name w:rFonts/@w:ascii; write the + # short spelling once. + def yaml_font_to(instance, doc) + yaml_put(doc, "font", instance.fonts&.ascii) end def yaml_font_ascii_from(instance, value) @@ -127,8 +139,8 @@ def yaml_font_ascii_from(instance, value) instance.fonts.ascii = value if value end - def yaml_font_ascii_to(_instance, _doc) - fonts&.ascii + def yaml_font_ascii_to(instance, doc) + yaml_font_to(instance, doc) end def yaml_font_east_asia_from(instance, value) @@ -136,8 +148,8 @@ def yaml_font_east_asia_from(instance, value) instance.fonts.east_asia = value if value end - def yaml_font_east_asia_to(_instance, _doc) - fonts&.east_asia + def yaml_font_east_asia_to(instance, doc) + yaml_put(doc, "font_east_asia", instance.fonts&.east_asia) end def yaml_font_h_ansi_from(instance, value) @@ -145,8 +157,8 @@ def yaml_font_h_ansi_from(instance, value) instance.fonts.h_ansi = value if value end - def yaml_font_h_ansi_to(_instance, _doc) - fonts&.h_ansi + def yaml_font_h_ansi_to(instance, doc) + yaml_put(doc, "font_h_ansi", instance.fonts&.h_ansi) end def yaml_font_cs_from(instance, value) @@ -154,8 +166,8 @@ def yaml_font_cs_from(instance, value) instance.fonts.cs = value if value end - def yaml_font_cs_to(_instance, _doc) - fonts&.cs + def yaml_font_cs_to(instance, doc) + yaml_put(doc, "font_cs", instance.fonts&.cs) end # --- Effect transforms --- @@ -164,40 +176,45 @@ def yaml_emboss_from(instance, value) instance.emboss = Properties::Emboss.new(value: value) unless value.nil? end - def yaml_emboss_to(_instance, _doc) - emboss&.value + def yaml_emboss_to(instance, doc) + yaml_put(doc, "emboss", instance.emboss&.on?) end def yaml_imprint_from(instance, value) instance.imprint = Properties::Imprint.new(value: value) unless value.nil? end - def yaml_imprint_to(_instance, _doc) - imprint&.value + def yaml_imprint_to(instance, doc) + yaml_put(doc, "imprint", instance.imprint&.on?) end def yaml_shadow_from(instance, value) instance.shadow = Properties::Shadow.new(value: value) unless value.nil? end - def yaml_shadow_to(_instance, _doc) - shadow&.value + def yaml_shadow_to(instance, doc) + yaml_put(doc, "shadow", instance.shadow&.on?) end + # The YAML key "outline" carries w:outlineLvl, not the w:outline + # toggle. Kept as-is so existing style YAML keeps loading. def yaml_outline_from(instance, value) instance.outline_level = Properties::OutlineLevel.new(value: value) unless value.nil? end - def yaml_outline_to(_instance, _doc) - outline_level.value if outline_level.is_a?(Properties::OutlineLevel) + def yaml_outline_to(instance, doc) + level = instance.outline_level + return unless level.is_a?(Properties::OutlineLevel) + + yaml_put(doc, "outline", level.value) end def yaml_vertical_align_from(instance, value) instance.vertical_align = Properties::VerticalAlign.new(value: value) if value end - def yaml_vertical_align_to(_instance, _doc) - vertical_align&.value + def yaml_vertical_align_to(instance, doc) + yaml_put(doc, "vertical_align", instance.vertical_align&.value) end end end diff --git a/lib/uniword/wordprocessingml/structured_document_tag/showing_placeholder_header.rb b/lib/uniword/wordprocessingml/structured_document_tag/showing_placeholder_header.rb index 204ad97a..8c823345 100644 --- a/lib/uniword/wordprocessingml/structured_document_tag/showing_placeholder_header.rb +++ b/lib/uniword/wordprocessingml/structured_document_tag/showing_placeholder_header.rb @@ -5,12 +5,23 @@ module Uniword module Wordprocessingml class StructuredDocumentTag - # Showing placeholder header flag for Structured Document Tag (empty element) - # Reference XML: + # Showing placeholder header flag for Structured Document Tag + # Reference XML: or + # + # This is an ST_OnOff toggle like every other one. It used to map no + # w:val at all, so an explicitly-off flag round-tripped as on and the + # four spellings were indistinguishable. class ShowingPlaceholderHeader < Lutaml::Model::Serializable + include Uniword::Properties::BooleanElement + + attribute :val, :string, default: nil + include Uniword::Properties::BooleanValSetter + xml do element "showingPlcHdr" namespace Ooxml::Namespaces::WordProcessingML + map_attribute "val", to: :val, render_nil: false, + render_default: false end end end diff --git a/lib/uniword/wordprocessingml/structured_document_tag/temporary.rb b/lib/uniword/wordprocessingml/structured_document_tag/temporary.rb index 54820dc0..a478d45b 100644 --- a/lib/uniword/wordprocessingml/structured_document_tag/temporary.rb +++ b/lib/uniword/wordprocessingml/structured_document_tag/temporary.rb @@ -5,13 +5,24 @@ module Uniword module Wordprocessingml class StructuredDocumentTag - # Temporary flag for Structured Document Tag (empty element) + # Temporary flag for Structured Document Tag # Indicates the SDT should be removed when content is first edited - # Reference XML: + # Reference XML: or + # + # This is an ST_OnOff toggle like every other one. It used to map no + # w:val at all, so an explicitly-off flag round-tripped as on and the + # six spellings were indistinguishable. class Temporary < Lutaml::Model::Serializable + include Uniword::Properties::BooleanElement + + attribute :val, :string, default: nil + include Uniword::Properties::BooleanValSetter + xml do element "temporary" namespace Ooxml::Namespaces::WordProcessingML + map_attribute "val", to: :val, render_nil: false, + render_default: false end end end diff --git a/lib/uniword/wordprocessingml/style.rb b/lib/uniword/wordprocessingml/style.rb index 72b750e6..f80ccbd7 100644 --- a/lib/uniword/wordprocessingml/style.rb +++ b/lib/uniword/wordprocessingml/style.rb @@ -9,6 +9,8 @@ module Wordprocessingml # Generated from OOXML schema: wordprocessingml.yml # Element: class Style < Lutaml::Model::Serializable + include YamlWriter + # Pattern 0: ATTRIBUTES FIRST attribute :type, :string attribute :styleId, :string @@ -50,53 +52,57 @@ class Style < Lutaml::Model::Serializable end # YAML transform methods (instance methods called by lutaml-model's - # `with:` transform mechanism) + # `with:` transform mechanism). The `_from` readers build wrapper + # objects; the `_to` writers assign through YamlWriter#yaml_put rather + # than returning a value — see that module for why. def yaml_name_from(instance, value) instance.name = StyleName.new(val: value) if value end - def yaml_name_to(instance, _doc) - instance.name&.val + def yaml_name_to(instance, doc) + yaml_put(doc, "name", instance.name&.val) end def yaml_quick_format_from(instance, value) instance.qFormat = Properties::QuickFormat.new(value: value) unless value.nil? end - def yaml_quick_format_to(instance, _doc) - instance.qFormat&.value + # w:qFormat is an ST_OnOff toggle, read through BooleanElement#on? + # like every other one. + def yaml_quick_format_to(instance, doc) + yaml_put(doc, "quick_format", instance.qFormat&.on?) end def yaml_based_on_from(instance, value) instance.basedOn = BasedOn.new(val: value) if value end - def yaml_based_on_to(instance, _doc) - instance.basedOn&.val + def yaml_based_on_to(instance, doc) + yaml_put(doc, "based_on", instance.basedOn&.val) end def yaml_next_style_from(instance, value) instance.nextStyle = Next.new(val: value) if value end - def yaml_next_style_to(instance, _doc) - instance.nextStyle&.val + def yaml_next_style_to(instance, doc) + yaml_put(doc, "next_style", instance.nextStyle&.val) end def yaml_linked_style_from(instance, value) instance.link = Link.new(val: value) if value end - def yaml_linked_style_to(instance, _doc) - instance.link&.val + def yaml_linked_style_to(instance, doc) + yaml_put(doc, "linked_style", instance.link&.val) end def yaml_ui_priority_from(instance, value) instance.uiPriority = UiPriority.new(val: value.to_s) if value end - def yaml_ui_priority_to(instance, _doc) - instance.uiPriority&.val&.to_i + def yaml_ui_priority_to(instance, doc) + yaml_put(doc, "ui_priority", instance.uiPriority&.val&.to_i) end xml do @@ -152,11 +158,7 @@ def ui_priority end def quick_format - val = qFormat - return true if val == true - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + boolean_flag(qFormat) == true end def spacing_before @@ -172,23 +174,11 @@ def alignment end def keep_next - return false unless pPr - - val = pPr.keep_next_wrapper - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + boolean_flag(pPr&.keep_next_wrapper) == true end def keep_lines - return false unless pPr - - val = pPr.keep_lines_wrapper - return false if val.nil? - - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + boolean_flag(pPr&.keep_lines_wrapper) == true end def outline_level @@ -200,13 +190,11 @@ def font_family end def bold - return nil unless rPr - - val = rPr.bold - return nil if val.nil? + boolean_flag(rPr&.bold) + end - val = val.value if val.is_a?(Uniword::Properties::BooleanElement) - val == true + def italic + boolean_flag(rPr&.italic) end def font_size @@ -252,6 +240,18 @@ def paragraph_properties def run_properties rPr end + + private + + # ST_OnOff toggles arrive as BooleanElement wrappers when parsed and as + # plain booleans when built. Returns nil when the toggle is absent, so + # readers wanting a strict false-when-absent contract compare == true. + def boolean_flag(element) + return nil if element.nil? + return element.on? if element.is_a?(Uniword::Properties::BooleanElement) + + element == true + end end end end diff --git a/lib/uniword/wordprocessingml/update_fields.rb b/lib/uniword/wordprocessingml/update_fields.rb index 20b8b3c0..4280318e 100644 --- a/lib/uniword/wordprocessingml/update_fields.rb +++ b/lib/uniword/wordprocessingml/update_fields.rb @@ -8,19 +8,25 @@ module Wordprocessingml # # Element: # - # When true, Word updates all fields (TOC, page numbers, cross + # When on, Word updates all fields (TOC, page numbers, cross # references) the first time the document is opened — generated # documents show correct field values without a manual F9. + # + # This reads through the same ST_OnOff element machinery as every other + # toggle. It used to go through Ooxml::Types::OoxmlBoolean, a second + # definition that raised on a malformed w:val and disagreed about nil. class UpdateFields < Lutaml::Model::Serializable - attribute :value, Ooxml::Types::OoxmlBoolean, - default: -> { true } + include Uniword::Properties::BooleanElement + + attribute :val, :string, default: nil + include Uniword::Properties::BooleanValSetter xml do element "updateFields" namespace Uniword::Ooxml::Namespaces::WordProcessingML - map_attribute "val", to: :value, render_nil: false, - render_default: false + map_attribute "val", to: :val, render_nil: false, + render_default: false end end end diff --git a/lib/uniword/wordprocessingml/yaml_writer.rb b/lib/uniword/wordprocessingml/yaml_writer.rb new file mode 100644 index 00000000..a04e4d78 --- /dev/null +++ b/lib/uniword/wordprocessingml/yaml_writer.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Uniword + module Wordprocessingml + # Internal helper for the models that map YAML keys through `with:` + # transforms. + # + # lutaml-model hands a `to:` method the accumulating hash and throws its + # return value away, so every writer has to assign into that hash. + # Returning the value instead made to_yaml emit "--- {}" for a fully + # populated model. + module YamlWriter + # Write one YAML key, skipping the ones this model does not set. + # + # @param doc [Hash] Accumulating YAML hash + # @param key [String] YAML key to write + # @param value [Object, nil] Value, or nil to leave the key out + # @return [void] + def yaml_put(doc, key, value) + doc[key] = value unless value.nil? + end + end + end +end diff --git a/spec/uniword/accessibility/accessibility_checker_spec.rb b/spec/uniword/accessibility/accessibility_checker_spec.rb index f20a8d80..1fdbac87 100644 --- a/spec/uniword/accessibility/accessibility_checker_spec.rb +++ b/spec/uniword/accessibility/accessibility_checker_spec.rb @@ -128,12 +128,18 @@ end context "with violations" do - let(:image_without_alt) do - double("Image", alt_text: nil) - end - - before do - allow(document).to receive(:images).and_return([image_without_alt]) + # A picture whose wp:docPr carries no descr, so it has no alt text. + let(:document) do + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML) + + + + + + + XML end it "collects violations from rules" do diff --git a/spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb b/spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb index 674919a4..d958496c 100644 --- a/spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb +++ b/spec/uniword/accessibility/rules/image_alt_text_rule_spec.rb @@ -18,12 +18,50 @@ } end - describe "#check" do - let(:document) { double("Document") } + # Alt text for a w:drawing lives in wp:docPr/@descr. Build the documents by + # parsing real OOXML so the rule is exercised against the drawings that + # DocumentRoot#images actually returns. + def document_with(*bodies) + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML) + + + #{bodies.join} + + XML + end + + # A w:p holding one inline drawing. Pass alt_text: nil to omit @descr, which + # is how Word writes a picture that has no alternative text at all. + def drawing_paragraph(alt_text, id: 1) + descr = alt_text.nil? ? "" : %( descr="#{alt_text}") + <<~XML + + + + + + + + XML + end + + def document_with_alt_texts(*alt_texts) + paragraphs = alt_texts.each_with_index.map do |alt_text, index| + drawing_paragraph(alt_text, id: index + 1) + end + document_with(*paragraphs) + end + describe "#check" do context "with no images" do - before do - allow(document).to receive(:images).and_return([]) + let(:document) do + document_with("No pictures here") end it "returns no violations" do @@ -32,11 +70,7 @@ end context "with image missing alt text" do - let(:image_no_alt) { double("Image", alt_text: nil) } - - before do - allow(document).to receive(:images).and_return([image_no_alt]) - end + let(:document) { document_with_alt_texts(nil) } it "returns violation" do violations = rule.check(document) @@ -57,14 +91,29 @@ violation = rule.check(document).first expect(violation.suggestion).to include("descriptive alternative text") end + + it "reports the offending drawing as the element" do + violation = rule.check(document).first + expect(violation.element).to be_a(Uniword::Wordprocessingml::Drawing) + end end - context "with image having empty alt text" do - let(:image_empty_alt) { double("Image", alt_text: " ") } + context "with alt text padded by whitespace" do + # Padding is not description. If extract_alt_text returned the raw + # attribute, the length checks would measure the padding and a + # 3-character description would satisfy min_length: 10. + let(:document) { document_with_alt_texts(" img ") } + + it "measures the stripped length, not the padding" do + violation = rule.check(document).first - before do - allow(document).to receive(:images).and_return([image_empty_alt]) + expect(violation.message).to include("too short") + expect(violation.message).to include("3 chars") end + end + + context "with image having empty alt text" do + let(:document) { document_with_alt_texts(" ") } it "returns violation" do violations = rule.check(document) @@ -73,26 +122,69 @@ end context "with valid alt text" do - let(:image_with_alt) do - double("Image", alt_text: "A beautiful sunset over mountains") + let(:document) do + document_with_alt_texts("A beautiful sunset over mountains") end - before do - allow(document).to receive(:images).and_return([image_with_alt]) + it "returns no violations" do + expect(rule.check(document)).to be_empty + end + end + + context "with an anchored drawing" do + def anchored_document(descr) + attr = descr.nil? ? "" : %( descr="#{descr}") + document_with(<<~XML) + + + + + XML end - it "returns no violations" do + it "reads alt text from the anchor's docPr" do + document = anchored_document("A chart of quarterly revenue") + expect(rule.check(document)).to be_empty end + + it "reports missing alt text when the anchor has no descr" do + violation = rule.check(anchored_document(nil)).first + + expect(violation.message).to include("Image 1 missing alternative text") + end + end + + # CT_Drawing is a choice over wp:inline and wp:anchor with maxOccurs + # unbounded, so a drawing carrying both is schema-valid. + context "with a drawing carrying both inline and anchor" do + def both_frames_document(inline_attr) + document_with(<<~XML) + + + + + + + + + XML + end + + it "falls back to the anchor when the inline docPr has no descr" do + expect(rule.check(both_frames_document(""))).to be_empty + end + + # A blank descr is truthy in Ruby, so a naive || chain stops here and + # reports missing alt text despite the anchor carrying a real one. + it "falls back to the anchor when the inline descr is blank" do + expect(rule.check(both_frames_document(%( descr="")))).to be_empty + end end context "when check_quality is enabled" do context "with alt text too short" do - let(:image_short_alt) { double("Image", alt_text: "Logo") } - - before do - allow(document).to receive(:images).and_return([image_short_alt]) - end + let(:document) { document_with_alt_texts("Logo") } it "returns warning violation" do violations = rule.check(document) @@ -108,12 +200,7 @@ end context "with alt text too long" do - let(:long_text) { "a" * 200 } - let(:image_long_alt) { double("Image", alt_text: long_text) } - - before do - allow(document).to receive(:images).and_return([image_long_alt]) - end + let(:document) { document_with_alt_texts("a" * 200) } it "returns warning violation" do violations = rule.check(document) @@ -137,11 +224,7 @@ "picture of", ].each do |generic_text| context "with '#{generic_text}'" do - let(:image_generic) { double("Image", alt_text: generic_text) } - - before do - allow(document).to receive(:images).and_return([image_generic]) - end + let(:document) { document_with_alt_texts(generic_text) } it "returns warning for generic text" do violations = rule.check(document) @@ -154,12 +237,8 @@ end context "with good alt text" do - let(:image_good) do - double("Image", alt_text: "Company logo showing blue mountain") - end - - before do - allow(document).to receive(:images).and_return([image_good]) + let(:document) do + document_with_alt_texts("Company logo showing blue mountain") end it "returns no violations" do @@ -169,24 +248,9 @@ end context "when check_quality is disabled" do - let(:config_no_quality) do - { - wcag_criterion: "1.1.1 Non-text Content", - level: "A", - enabled: true, - severity: :error, - check_quality: false, - min_length: 10, - max_length: 150, - suggestion: "Add descriptive alternative text", - } - end + let(:config_no_quality) { config.merge(check_quality: false) } let(:rule_no_quality) { described_class.new(config_no_quality) } - let(:image_short) { double("Image", alt_text: "Logo") } - - before do - allow(document).to receive(:images).and_return([image_short]) - end + let(:document) { document_with_alt_texts("Logo") } it "does not check quality" do violations = rule_no_quality.check(document) @@ -195,12 +259,8 @@ end context "with multiple images" do - let(:image1) { double("Image", alt_text: nil) } - let(:image2) { double("Image", alt_text: "Valid description here") } - let(:image3) { double("Image", alt_text: "img") } - - before do - allow(document).to receive(:images).and_return([image1, image2, image3]) + let(:document) do + document_with_alt_texts(nil, "Valid description here", "img") end it "checks all images" do diff --git a/spec/uniword/accessibility/rules_integration_spec.rb b/spec/uniword/accessibility/rules_integration_spec.rb index 3339f342..d478aeaa 100644 --- a/spec/uniword/accessibility/rules_integration_spec.rb +++ b/spec/uniword/accessibility/rules_integration_spec.rb @@ -10,6 +10,20 @@ allow(document).to receive_messages(images: [], tables: [], paragraphs: []) end + # A picture whose wp:docPr carries no descr, so it has no alt text. + def document_with_alt_less_picture + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML) + + + + + + + XML + end + describe "All rules can be instantiated" do let(:config) do { @@ -83,8 +97,7 @@ describe "Rules return violations when issues found" do it "ImageAltTextRule detects missing alt text" do - image_no_alt = double("Image", alt_text: nil) - allow(document).to receive(:images).and_return([image_no_alt]) + document = document_with_alt_less_picture rule = Uniword::Accessibility::Rules::ImageAltTextRule.new( wcag_criterion: "1.1.1", @@ -133,9 +146,6 @@ describe "Rules respect enabled flag" do it "disabled rules return no violations" do - image_no_alt = double("Image", alt_text: nil) - allow(document).to receive(:images).and_return([image_no_alt]) - rule = Uniword::Accessibility::Rules::ImageAltTextRule.new( wcag_criterion: "1.1.1", level: "A", diff --git a/spec/uniword/accessibility/shipped_profile_alt_text_spec.rb b/spec/uniword/accessibility/shipped_profile_alt_text_spec.rb new file mode 100644 index 00000000..f470240d --- /dev/null +++ b/spec/uniword/accessibility/shipped_profile_alt_text_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Alt text checked the way users actually get it: through the shipped +# wcag_2_1_aa profile (check_quality: true, require_alt_text: true, +# min_length: 10, max_length: 150) and through Quality::DocumentChecker's +# shipped rules. A rule configured by hand in a spec proves nothing about +# what ships. +templates = %w[ + word-template-apa-style-paper + word-template-mla-style-paper + word-template-paper-with-cover-and-toc +].freeze + +RSpec.describe "image alt text through the shipped configuration" do + def fixture(name) + File.join(__dir__, "../../fixtures", name, "#{name}.docx") + end + + def document(name) + Uniword::Docx::Package.from_file(fixture(name)).document + end + + def accessibility_image_violations(doc) + Uniword::Accessibility::AccessibilityChecker.new + .check(doc).violations + .map(&:message).grep(/\AImage \d+ /) + end + + def quality_image_violations(doc) + Uniword::Quality::DocumentChecker.new + .check(doc).violations + .map(&:message).grep(/\AImage \d+ /) + end + + # These three Microsoft templates each ship one picture whose description + # sits in docPr/@title, with no @descr at all. Title is a caption, so the + # image is undescribed and both checkers must say so — and say the same + # number of times. + templates.each do |name| + context name do + let(:doc) { document(name) } + + it "reports the title-only picture as missing alt text" do + expect(accessibility_image_violations(doc)) + .to eq(["Image 1 missing alternative text"]) + end + + it "makes the quality checker agree with the accessibility checker" do + expect(quality_image_violations(doc).size) + .to eq(accessibility_image_violations(doc).size) + end + + it "never calls a caption in @title generic alt text" do + expect(accessibility_image_violations(doc)) + .to all(satisfy { |m| !m.include?("generic alt text") }) + end + + it "points the author at the title field it found the text in" do + suggestions = Uniword::Accessibility::AccessibilityChecker.new + .check(doc).violations + .select { |v| v.message.to_s.start_with?("Image 1 ") } + .map(&:suggestion) + + expect(suggestions).to all(include("title")) + end + end + end + + describe "a picture that does carry a description" do + let(:doc) do + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML) + + + + + + + + XML + end + + it "passes both checkers" do + aggregate_failures do + expect(accessibility_image_violations(doc)).to be_empty + expect(quality_image_violations(doc)).to be_empty + end + end + end + + describe "an image inside a table cell" do + let(:doc) do + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML) + + + + + + + + + + XML + end + + # The renderer emits a table-nested picture, so a checker that walks only + # top-level paragraphs reports a clean document that is not clean. + it "is visible to both checkers" do + aggregate_failures do + expect(doc.images.size).to eq(1) + expect(accessibility_image_violations(doc).size).to eq(1) + expect(quality_image_violations(doc).size).to eq(1) + end + end + end +end diff --git a/spec/uniword/builder/image_alt_text_spec.rb b/spec/uniword/builder/image_alt_text_spec.rb new file mode 100644 index 00000000..c6e32ce3 --- /dev/null +++ b/spec/uniword/builder/image_alt_text_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require "spec_helper" + +# The writing side of alt text. ImageBuilder took an alt_text: argument and +# threw it away, so every picture the builder produced was undescribed. +RSpec.describe "building an image with alt text" do + let(:png) { File.join(__dir__, "../../fixtures/sample.png") } + let(:builder) { Uniword::Builder::DocumentBuilder.new } + + def drawing(alt_text: nil, floating: false) + if floating + Uniword::Builder::ImageBuilder.create_floating( + builder, png, alt_text: alt_text + ) + else + Uniword::Builder::ImageBuilder.create_drawing( + builder, png, alt_text: alt_text + ) + end + end + + describe "an inline image" do + it "carries the alt text into docPr/@descr" do + d = drawing(alt_text: "A red square on white") + + aggregate_failures do + expect(d.alt_text).to eq("A red square on white") + expect(d.to_xml).to include('descr="A red square on white"') + end + end + + it "writes no descr when no alt text was given" do + d = drawing + + aggregate_failures do + expect(d.alt_text).to be_nil + expect(d.to_xml).not_to include("descr=") + end + end + end + + describe "a floating image" do + it "carries the alt text into docPr/@descr" do + d = drawing(alt_text: "A floating logo", floating: true) + + aggregate_failures do + expect(d.alt_text).to eq("A floating logo") + expect(d.to_xml).to include('descr="A floating logo"') + end + end + end + + describe "DocumentBuilder#image and #floating_image" do + it "passes alt text through to the drawing" do + builder.image(png, alt_text: "Inline described") + builder.floating_image(png, alt_text: "Floating described") + + expect(builder.model.images.map(&:alt_text)) + .to eq(["Inline described", "Floating described"]) + end + end +end diff --git a/spec/uniword/builder/sdt_builder_toggles_spec.rb b/spec/uniword/builder/sdt_builder_toggles_spec.rb new file mode 100644 index 00000000..fe4c1141 --- /dev/null +++ b/spec/uniword/builder/sdt_builder_toggles_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Both builder methods took a documented Boolean and threw it away — the +# parameter was named `_value` and the toggle was always constructed bare, +# which ST_OnOff reads as ON. So `lock(false)` produced a locked control and +# `showing_placeholder(false)` produced a placeholder, each the opposite of +# what the caller asked for. +RSpec.describe Uniword::Builder::SdtBuilder do + subject(:builder) { described_class.new } + + def sdt_pr_xml + builder.build.properties.to_xml + end + + describe "#lock" do + it "writes a bare w:temporary when locked" do + builder.lock(true) + + expect(sdt_pr_xml).to match(%r{<(w:)?temporary\s*/>}) + end + + it "writes w:val=\"false\" when explicitly not locked" do + builder.lock(false) + + expect(sdt_pr_xml).to match(/val="false"/) + end + + it "defaults to locked" do + builder.lock + + expect(builder.properties.temporary.on?).to be(true) + end + + it "round-trips the value it was given" do + expect([true, false].map { |v| described_class.new.lock(v).properties.temporary.on? }) + .to eq([true, false]) + end + end + + describe "#showing_placeholder" do + it "writes a bare w:showingPlcHdr when shown" do + builder.showing_placeholder(true) + + expect(sdt_pr_xml).to match(%r{<(w:)?showingPlcHdr\s*/>}) + end + + it "writes w:val=\"false\" when explicitly not shown" do + builder.showing_placeholder(false) + + expect(sdt_pr_xml).to match(/val="false"/) + end + + it "defaults to shown" do + builder.showing_placeholder + + expect(builder.properties.showing_placeholder_header.on?).to be(true) + end + + it "round-trips the value it was given" do + expect([true, false].map do |v| + described_class.new.showing_placeholder(v).properties + .showing_placeholder_header.on? + end).to eq([true, false]) + end + end + + # The renderer is the consumer that actually acted on the wrong reading. + describe "through the mhtml renderer" do + it "does not emit the flags a caller explicitly turned off" do + props = described_class.new.lock(false).showing_placeholder(false) + .build.properties + html = Uniword::Transformation::MhtmlElementRenderer.new + .send(:build_sdt_attrs, props) + + expect(html).not_to include("temporary") + expect(html).not_to include("showingPlcHdr") + end + + it "emits both flags a caller turned on" do + props = described_class.new.lock(true).showing_placeholder(true) + .build.properties + html = Uniword::Transformation::MhtmlElementRenderer.new + .send(:build_sdt_attrs, props) + + expect(html).to include('w:temporary="t"') + expect(html).to include('w:showingPlcHdr="t"') + end + end +end diff --git a/spec/uniword/images/docpr_id_uniqueness_spec.rb b/spec/uniword/images/docpr_id_uniqueness_spec.rb new file mode 100644 index 00000000..04e0d06a --- /dev/null +++ b/spec/uniword/images/docpr_id_uniqueness_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require "spec_helper" +require "tmpdir" +require "zip" + +# This file is a guard, not coverage of any one implementation. An id +# allocator was once written to solve the problem below and then reverted in +# favour of the existing deterministic scheme, which already satisfies it — +# so these examples pass both with and without that allocator, by design. +# They exist to fail if a future change reintroduces colliding ids. +# +# ECMA-376 §20.4.2.5 requires wp:docPr/@id unique inside a part. Word treats a +# duplicate as a repair-triggering error, so an id scheme that counts from 1 +# collides with the ids a template already carries. +# +# The APA template ships with one drawing whose docPr id is 2, which makes it +# the cheapest fixture to catch that. +RSpec.describe "wp:docPr ids when editing a document that already has drawings" do + let(:template) do + File.join(__dir__, "../../fixtures/word-template-apa-style-paper", + "word-template-apa-style-paper.docx") + end + let(:png) { File.join(__dir__, "../../fixtures/sample.png") } + let(:other_png) { File.join(__dir__, "../../fixtures/docx_gem/replacement.png") } + + def doc_pr_ids(root) + root.images + .filter_map { |drawing| (drawing.inline || drawing.anchor)&.doc_properties&.id } + .map(&:to_s) + end + + it "does not reuse an id the template already spent" do + root = Uniword.load(template) + expect(doc_pr_ids(root)).to include("2") + + manager = Uniword::Images::ImageManager.new(root) + manager.insert(png, description: "first") + manager.insert(other_png, description: "second") + + ids = doc_pr_ids(root) + expect(ids.uniq.size).to eq(ids.size) + end + + it "gives two floating images of different files different ids" do + root = Uniword.load(template) + + ids = [png, other_png].map do |path| + Uniword::Builder::ImageBuilder + .create_floating(root, path).anchor.doc_properties.id + end + + expect(ids.uniq.size).to eq(2) + end + + it "writes unique ids all the way to the saved package" do + root = Uniword.load(template) + manager = Uniword::Images::ImageManager.new(root) + manager.insert(png, description: "first") + manager.insert(other_png, description: "second") + + Dir.mktmpdir do |dir| + out = File.join(dir, "edited.docx") + Uniword::DocumentWriter.new(root).save(out, validate: false) + + xml = Zip::File.open(out) { |zip| zip.read("word/document.xml") } + ids = xml.scan(/]*\bid="([^"]+)"/).flatten + + expect(ids.size).to be >= 3 + expect(ids.uniq.size).to eq(ids.size) + end + end +end diff --git a/spec/uniword/mhtml/word_css_spec.rb b/spec/uniword/mhtml/word_css_spec.rb index ccef4291..e8531615 100644 --- a/spec/uniword/mhtml/word_css_spec.rb +++ b/spec/uniword/mhtml/word_css_spec.rb @@ -63,48 +63,114 @@ end end + # WordCss consumes a WordprocessingML styles configuration: generate_style_css + # calls styles_config.styles.map and then reads style-object methods, while + # Mhtml::StylesConfiguration#styles is a plain hash of CSS properties. + def styles_config_from(*style_xml) + Uniword::Wordprocessingml::StylesConfiguration.from_xml(<<~XML) + + #{style_xml.join} + + XML + end + + # Note w:sz is in half-points, so w:val="24" is a 12pt font. + def style_xml(id, **opts) + [ + %(), + paragraph_props_xml(opts[:align]), + run_props_xml(opts), + "", + ].join + end + + def paragraph_props_xml(align) + return "" unless align + + %() + end + + def run_props_xml(opts) + parts = [ + (%() if opts[:font]), + ("" if opts[:bold]), + ("" if opts[:italic]), + (%() if opts[:half_points]), + ].compact + return "" if parts.empty? + + "#{parts.join}" + end + + def single_style(xml) + styles_config_from(xml).styles.first + end + + # A style whose w:b and w:i carry explicit ST_OnOff values. + def toggle_style(bold_val, italic_val) + single_style(<<~XML) + + + + + XML + end + + # generate_list_css calls numbering_config.instances, which only + # Wordprocessingml::NumberingConfiguration declares. + def numbering_config_from(*num_ids) + nums = num_ids.map { |id| %() }.join + Uniword::Wordprocessingml::NumberingConfiguration.from_xml(<<~XML) + + #{nums} + + XML + end + describe ".generate_style_css" do it "returns empty string for nil config" do css = described_class.generate_style_css(nil) expect(css).to eq("") end - it "generates CSS for styles" do - # Create a mock style - style = double("Style", - style_id: "CustomStyle", - font: "Arial", - font_size: 12, - bold: true, - italic: false, - alignment: "center") + it "generates CSS from a parsed WordprocessingML styles configuration" do + config = styles_config_from( + style_xml("RichStyle", font: "Arial", bold: true, italic: true, + align: "right"), + ) + + expected = <<~CSS.chomp + .RichStyle { + font-family: 'Arial'; + font-weight: bold; + font-style: italic; + text-align: right; + } + CSS + + expect(described_class.generate_style_css(config)).to eq(expected) + end - config = double("StylesConfiguration") - allow(config).to receive(:styles).and_return([style]) + it "handles styles without optional properties" do + config = styles_config_from(style_xml("Simple")) css = described_class.generate_style_css(config) - expect(css).to include(".CustomStyle") - expect(css).to include("Arial") - expect(css).to include("12pt") - expect(css).to include("bold") - expect(css).to include("center") - end - it "handles styles without optional properties" do - style = double("Style", - style_id: "Simple", - font: nil, - font_size: nil, - bold: nil, - italic: nil, - alignment: nil) + expect(css).to eq("") + end - config = double("StylesConfiguration") - allow(config).to receive(:styles).and_return([style]) + it "generates one rule per style" do + config = styles_config_from( + style_xml("First", font: "Arial"), + style_xml("Second", font: "Georgia"), + ) css = described_class.generate_style_css(config) - # Should not raise error, may return empty or minimal CSS - expect(css).to be_a(String) + + expect(css).to include(".First") + expect(css).to include(".Second") end end @@ -115,27 +181,19 @@ end it "generates CSS for numbering" do - instance = double("NumberingInstance", num_id: 1) - - config = double("NumberingConfiguration") - allow(config).to receive(:instances).and_return([instance]) + config = numbering_config_from(1) css = described_class.generate_list_css(config) + expect(css).to include("@list l1") expect(css).to include("mso-list-id: 1") end it "handles multiple numbering instances" do - instances = [ - double("NumberingInstance", num_id: 1), - double("NumberingInstance", num_id: 2), - double("NumberingInstance", num_id: 3), - ] - - config = double("NumberingConfiguration") - allow(config).to receive(:instances).and_return(instances) + config = numbering_config_from(1, 2, 3) css = described_class.generate_list_css(config) + expect(css).to include("@list l1") expect(css).to include("@list l2") expect(css).to include("@list l3") @@ -149,29 +207,22 @@ end it "builds CSS rule for style with font" do - style = double("Style", - style_id: "TestStyle", - font: "Times New Roman", - font_size: nil, - bold: nil, - italic: nil, - alignment: nil) + style = single_style(style_xml("TestStyle", font: "Times New Roman")) rule = described_class.build_style_rule(style) + expect(rule).to include(".TestStyle") expect(rule).to include("Times New Roman") end it "builds CSS rule for style with multiple properties" do - style = double("Style", - style_id: "RichStyle", - font: "Arial", - font_size: 14, - bold: true, - italic: true, - alignment: "right") + style = single_style( + style_xml("RichStyle", font: "Arial", half_points: 28, bold: true, + italic: true, align: "right"), + ) rule = described_class.build_style_rule(style) + expect(rule).to include(".RichStyle") expect(rule).to include("Arial") expect(rule).to include("14pt") @@ -180,18 +231,37 @@ expect(rule).to include("right") end + it "converts the half-point w:sz value to points" do + style = single_style(style_xml("Sized", half_points: 24)) + + expect(described_class.build_style_rule(style)) + .to include("font-size: 12pt") + end + it "returns nil for style with no properties" do - style = double("Style", - style_id: "EmptyStyle", - font: nil, - font_size: nil, - bold: nil, - italic: nil, - alignment: nil) + style = single_style(style_xml("EmptyStyle")) rule = described_class.build_style_rule(style) + expect(rule).to be_nil end + + # An ST_OnOff false token must not turn the toggle on. rFonts keeps the + # rule non-nil so the absence assertions have a string to run against. + it "omits toggles whose w:val is an ST_OnOff false token" do + rule = described_class.build_style_rule(toggle_style("0", "0")) + + expect(rule).not_to include("font-weight: bold") + expect(rule).not_to include("font-style: italic") + end + + it "returns nil for a style with no styleId" do + style = single_style( + %(), + ) + + expect(described_class.build_style_rule(style)).to be_nil + end end describe ".build_list_rule" do @@ -201,7 +271,7 @@ end it "builds @list rule" do - instance = double("NumberingInstance", num_id: 5) + instance = numbering_config_from(5).instances.first rule = described_class.build_list_rule(instance) expect(rule).to include("@list l5") diff --git a/spec/uniword/ooxml/types/ooxml_boolean_optional_spec.rb b/spec/uniword/ooxml/types/ooxml_boolean_optional_spec.rb index 9bdf1588..f4b3dd9c 100644 --- a/spec/uniword/ooxml/types/ooxml_boolean_optional_spec.rb +++ b/spec/uniword/ooxml/types/ooxml_boolean_optional_spec.rb @@ -18,9 +18,8 @@ expect(described_class.cast(nil)).to be_nil end - it "raises on an unknown value instead of passing it through" do - expect { described_class.cast("yes") } - .to raise_error(Lutaml::Model::Type::InvalidValueError) + it "reads an unknown token as on instead of raising" do + expect(described_class.cast("yes")).to be(true) end end diff --git a/spec/uniword/ooxml/types/ooxml_boolean_spec.rb b/spec/uniword/ooxml/types/ooxml_boolean_spec.rb index eb522db6..24bb9a09 100644 --- a/spec/uniword/ooxml/types/ooxml_boolean_spec.rb +++ b/spec/uniword/ooxml/types/ooxml_boolean_spec.rb @@ -18,9 +18,17 @@ expect(described_class.cast(nil)).to be false end - it "raises on an unknown value instead of passing it through" do - expect { described_class.cast("diagonal-garbage") } - .to raise_error(Lutaml::Model::Type::InvalidValueError) + # A reader must not raise on a malformed document. One bad token in + # styles.xml used to kill the whole parse. + it "reads an unknown token as on instead of raising" do + expect(described_class.cast("diagonal-garbage")).to be(true) + end + + it "reads the same way Properties::BooleanElement does" do + %w[1 0 true false on off banana].each do |token| + expect(described_class.cast(token)) + .to be(Uniword::Properties::Bold.new(val: token).on?) + end end end diff --git a/spec/uniword/properties/bold_spec.rb b/spec/uniword/properties/bold_spec.rb index 3610bb48..5f6d86df 100644 --- a/spec/uniword/properties/bold_spec.rb +++ b/spec/uniword/properties/bold_spec.rb @@ -43,6 +43,36 @@ bold = described_class.from_xml(xml) expect(bold.to_xml).to include('w:val="false"') end + + # ST_OnOff (ECMA-376) accepts 0/1, false/true and off/on. An absent w:val + # means the toggle is on. Bold stands in for every BooleanElement + # includer; this is the one home for that table. + describe "#on?" do + { + nil => true, + "0" => false, + "false" => false, + "off" => false, + "1" => true, + "true" => true, + "on" => true, + }.each do |val, expected| + it "reads #{val.inspect} as #{expected}" do + attr = val.nil? ? "" : %( w:val="#{val}") + bold = described_class.from_xml("") + + expect(bold.on?).to be(expected) + end + end + + # Unknown tokens stay on, matching the #value beside it. Reading is + # total: a malformed attribute must not raise out of a reader. + it "reads an unknown token as on" do + bold = described_class.from_xml(%()) + + expect(bold.on?).to be(true) + end + end end describe Uniword::Properties::BoldCs do diff --git a/spec/uniword/properties/boolean_element_spec.rb b/spec/uniword/properties/boolean_element_spec.rb new file mode 100644 index 00000000..24b7e632 --- /dev/null +++ b/spec/uniword/properties/boolean_element_spec.rb @@ -0,0 +1,214 @@ +# frozen_string_literal: true + +require "spec_helper" + +# ST_OnOff (ECMA-376 §17.17.4) spells off as "0", "false" or "off". Word +# shows all three as off. Every reader in the library has to agree on that, +# and so does every writer. +ns_decl = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + +# label => [written spelling, expected reading] +spellings = { + "absent" => [nil, true], + "1" => ["1", true], + "0" => ["0", false], + "true" => ["true", true], + "false" => ["false", false], + "on" => ["on", true], + "off" => ["off", false], + "empty" => ["", true], + "garbage" => ["banana", true], + "integer 1" => [1, true], + "integer 0" => [0, false], + "Ruby true" => [true, true], + "Ruby false" => [false, false], +}.freeze + +# Every rPr toggle, as [attribute, element name, predicate or nil]. +# +# This list must cover Conversion::BOOLEAN_WRAPPERS exactly. It used to name +# ten of the fifteen, and the five it skipped — bold_cs, italic_cs, +# double_strike, no_proof, web_hidden — were the ones whose conversion could +# be deleted without a single spec noticing. +toggles = [ + [:bold, "b", :bold?], + [:bold_cs, "bCs", nil], + [:italic, "i", :italic?], + [:italic_cs, "iCs", nil], + [:strike, "strike", :strike?], + [:double_strike, "dstrike", nil], + [:small_caps, "smallCaps", :small_caps?], + [:caps, "caps", :caps?], + [:hidden, "vanish", :hidden?], + [:no_proof, "noProof", nil], + [:web_hidden, "webHidden", nil], + [:shadow, "shadow", :shadow?], + [:emboss, "emboss", :emboss?], + [:imprint, "imprint", :imprint?], + [:outline, "outline", :outline?], +].freeze + +RSpec.describe Uniword::Properties::BooleanElement do + # A toggle with no predicate method is still read the same way; ask the + # wrapper directly so the table can cover every attribute. + def reading(rpr, attr, predicate) + return rpr.public_send(predicate) if predicate + + rpr.public_send(attr)&.on? + end + + describe "the toggle list" do + it "covers every attribute Conversion wraps" do + wrapped = Uniword::Wordprocessingml::RunProperties::Conversion::BOOLEAN_WRAPPERS + expect(toggles.map(&:first)).to match_array(wrapped.keys) + end + + it "names the wrapper class each attribute converts to" do + wrapped = Uniword::Wordprocessingml::RunProperties::Conversion::BOOLEAN_WRAPPERS + toggles.each do |attr, _element, _predicate| + rpr = Uniword::Wordprocessingml::RunProperties.new(attr => "1") + expect(rpr.public_send(attr)).to be_a(wrapped.fetch(attr)) + end + end + end + + describe "reading a parsed toggle" do + toggles.each do |attr, element, predicate| + spellings.each do |label, (written, expected)| + next if [1, 0, true, false].include?(written) # XML carries strings + + it "reads #{label} as #{expected} through every reader" do + inner = written.nil? ? "" : %() + rpr = Uniword::Wordprocessingml::RunProperties + .from_xml(%(#{inner})) + wrapper = rpr.public_send(attr) + + aggregate_failures do + expect(wrapper.on?).to be(expected) + expect(wrapper.value).to be(expected) + expect(reading(rpr, attr, predicate)).to be(expected) + end + end + end + end + end + + describe "writing a toggle" do + toggles.each do |attr, _element, predicate| + spellings.each do |label, (written, expected)| + next if written.nil? + + it "writes #{attr} #{label} so it reads back as #{expected}" do + rpr = Uniword::Wordprocessingml::RunProperties.new(attr => written) + reparsed = Uniword::Wordprocessingml::RunProperties + .from_xml(rpr.to_xml) + + aggregate_failures do + expect(reading(rpr, attr, predicate)).to be(expected) + expect(reading(reparsed, attr, predicate)).to be(expected) + end + end + end + end + end + + describe "value:" do + spellings.each do |label, (written, expected)| + next if written.nil? + + it "treats value: #{label} as val: #{label}" do + by_value = Uniword::Properties::Bold.new(value: written) + by_val = Uniword::Properties::Bold.new(val: written) + + aggregate_failures do + expect(by_value.val).to eq(by_val.val) + expect(by_value.on?).to be(expected) + expect(by_value.to_xml).to eq(by_val.to_xml) + end + end + end + + it "lets an explicit val: win over value:" do + bold = Uniword::Properties::Bold.new(val: "0", value: "1") + expect(bold.on?).to be(false) + end + + it "accepts a string 'value' key" do + expect(Uniword::Properties::Bold.new("value" => "off").on?).to be(false) + end + end + + describe "#value=" do + it "assigns through to val" do + bold = Uniword::Properties::Bold.new + bold.value = "off" + expect(bold.val).to eq("off") + expect(bold.on?).to be(false) + end + + # value= has to go through val=, which is the setter that normalises a + # Ruby boolean into the ST_OnOff spelling. Writing the ivar behind its + # back leaves a bare false on the attribute, and then on? reads it as on + # and to_xml has no string to render. + it "normalises a Ruby boolean the way val= does" do + off = Uniword::Properties::Bold.new + off.value = false + on = Uniword::Properties::Bold.new + on.value = true + + aggregate_failures do + expect(off.val).to eq("false") + expect(off.on?).to be(false) + expect(off.to_xml).to include('w:val="false"') + expect(on.val).to be_nil + expect(on.on?).to be(true) + expect(on.to_xml).not_to include("w:val") + end + end + end + + describe "round trips" do + toggles.each do |attr, element, predicate| + spellings.each do |label, (written, expected)| + next if [1, 0, true, false].include?(written) + + it "keeps #{label} at #{expected} over two serialize cycles" do + inner = written.nil? ? "" : %() + first = Uniword::Wordprocessingml::RunProperties + .from_xml(%(#{inner})) + second = Uniword::Wordprocessingml::RunProperties.from_xml(first.to_xml) + third = Uniword::Wordprocessingml::RunProperties.from_xml(second.to_xml) + + aggregate_failures do + expect(reading(first, attr, predicate)).to be(expected) + expect(reading(second, attr, predicate)).to be(expected) + expect(reading(third, attr, predicate)).to be(expected) + end + end + end + end + end + + describe "a Ruby false handed to RunProperties" do + it "becomes an off toggle rather than a bare false" do + rpr = Uniword::Wordprocessingml::RunProperties.new(bold: false) + + aggregate_failures do + expect(rpr.bold).to be_a(Uniword::Properties::Bold) + expect(rpr.bold?).to be(false) + expect(rpr.to_xml).to include('w:val="false"') + end + end + + toggles.each do |attr, element, _predicate| + it "serializes #{attr} set to a Ruby false" do + rpr = Uniword::Wordprocessingml::RunProperties.new(attr => false) + + aggregate_failures do + expect { rpr.to_xml }.not_to raise_error + expect(rpr.to_xml).to match(%r{<(w:)?#{element} w:val="false"\s*/>}) + end + end + end + end +end diff --git a/spec/uniword/quality/rules/image_alt_text_reading_spec.rb b/spec/uniword/quality/rules/image_alt_text_reading_spec.rb new file mode 100644 index 00000000..7085c7ca --- /dev/null +++ b/spec/uniword/quality/rules/image_alt_text_reading_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require "spec_helper" + +# The quality rule is the second consumer of the one alt-text reading, and it +# walks the same drawing list as the accessibility rule. Both halves of that +# claim are asserted here: the rule reads through Drawing#alt_text rather than +# the raw docPr/@descr, and DocumentRoot#images reaches a picture sitting in a +# table nested inside a table cell. +RSpec.describe Uniword::Quality::ImageAltTextRule do + subject(:rule) { described_class.new(require_alt_text: true, min_length: 10) } + + let(:document) { Uniword::Wordprocessingml::DocumentRoot.new } + + before do + document.body ||= Uniword::Wordprocessingml::Body.new + document.body.paragraphs ||= [] + document.body.tables ||= [] + end + + def drawing_with(descr) + drawing = Uniword::Wordprocessingml::Drawing.new + inline = Uniword::WpDrawing::Inline.new + inline.doc_properties = Uniword::WpDrawing::DocProperties.new( + id: "1", name: "image1", descr: descr + ) + drawing.inline = inline + drawing + end + + def paragraph_with(drawing) + run = Uniword::Wordprocessingml::Run.new + run.drawings << drawing + paragraph = Uniword::Wordprocessingml::Paragraph.new + paragraph.runs << run + paragraph + end + + def cell_with(paragraphs: [], tables: []) + cell = Uniword::Wordprocessingml::TableCell.new + cell.paragraphs ||= [] + cell.tables ||= [] + paragraphs.each { |para| cell.paragraphs << para } + tables.each { |table| cell.tables << table } + cell + end + + def table_with(cell) + row = Uniword::Wordprocessingml::TableRow.new + row.cells ||= [] + row.cells << cell + table = Uniword::Wordprocessingml::Table.new + table.rows ||= [] + table.rows << row + table + end + + # A descr of nothing but spaces describes nothing. Drawing#alt_text strips + # it and calls it absent; the raw attribute is a three-character string that + # reads as present but too short. + describe "reading alt text" do + it "calls a whitespace-only descr missing, not short" do + document.body.paragraphs << paragraph_with(drawing_with(" ")) + + violations = rule.check(document) + + aggregate_failures do + expect(violations.size).to eq(1) + expect(violations.first.severity).to eq(:error) + expect(violations.first.message).to include("missing alt text") + end + end + + it "strips surrounding whitespace before measuring the length" do + document.body.paragraphs << paragraph_with(drawing_with(" #{'a' * 9} ")) + + violations = rule.check(document) + + aggregate_failures do + expect(violations.size).to eq(1) + expect(violations.first.severity).to eq(:warning) + expect(violations.first.message).to include("9 characters") + end + end + end + + describe "images inside tables" do + it "checks a picture in a table cell" do + cell = cell_with(paragraphs: [paragraph_with(drawing_with(nil))]) + document.body.tables << table_with(cell) + + expect(rule.check(document).size).to eq(1) + end + + it "checks a picture in a table nested inside a table cell" do + inner = table_with(cell_with(paragraphs: [paragraph_with(drawing_with(nil))])) + document.body.tables << table_with(cell_with(tables: [inner])) + + aggregate_failures do + expect(document.images.size).to eq(1) + expect(rule.check(document).size).to eq(1) + end + end + + it "leaves a described picture in a nested table alone" do + described = paragraph_with(drawing_with("A bar chart of yearly totals")) + inner = table_with(cell_with(paragraphs: [described])) + document.body.tables << table_with(cell_with(tables: [inner])) + + expect(rule.check(document)).to be_empty + end + end +end diff --git a/spec/uniword/transformation/image_alt_rendering_spec.rb b/spec/uniword/transformation/image_alt_rendering_spec.rb new file mode 100644 index 00000000..dbff440c --- /dev/null +++ b/spec/uniword/transformation/image_alt_rendering_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Alt text has to survive every hop: docPr/@descr -> Drawing#alt_text -> +# the rendered . +RSpec.describe "alt text in rendered output" do + templates = { + "word-template-apa-style-paper" => nil, + "word-template-mla-style-paper" => nil, + "word-template-paper-with-cover-and-toc" => nil, + }.freeze + + def package(name) + Uniword::Docx::Package.from_file( + File.join(__dir__, "../../fixtures", name, "#{name}.docx"), + ) + end + + def html_for(pkg) + Uniword::Transformation::Transformer.new + .docx_package_to_mhtml(pkg, "doc").raw_html.to_s + end + + describe "a real .docx whose pictures carry only a title" do + templates.each_key do |name| + it "renders #{name} images with no alt attribute" do + html = html_for(package(name)) + imgs = html.scan(/]*>/) + + aggregate_failures do + expect(imgs).not_to be_empty + # An empty alt claims the image is decorative. We have no grounds + # for that claim, so the attribute stays off entirely. + expect(imgs).to all(satisfy { |tag| !tag.include?("alt=") }) + end + end + end + end + + describe "a built document with alt text" do + let(:png) { File.join(__dir__, "../../fixtures/sample.png") } + + it "carries the description into the rendered img tag" do + builder = Uniword::Builder::DocumentBuilder.new + builder.image(png, alt_text: %(A "red" square & a leaf)) + + drawing = builder.model.images.first + renderer = Uniword::Transformation::MhtmlElementRenderer.new( + nil, builder.model.image_parts + ) + tag = renderer.drawing_to_html(drawing) + + aggregate_failures do + expect(drawing.alt_text).to eq(%(A "red" square & a leaf)) + expect(tag).to include("alt=") + expect(tag).to include("&") + expect(tag).to include(""") + end + end + end +end diff --git a/spec/uniword/transformation/ooxml_to_html_converter_spec.rb b/spec/uniword/transformation/ooxml_to_html_converter_spec.rb new file mode 100644 index 00000000..7108bf65 --- /dev/null +++ b/spec/uniword/transformation/ooxml_to_html_converter_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require "spec_helper" + +# This converter read the rPr toggles by object presence — `if props.bold` is +# true whenever the element exists, whatever its w:val says — so an +# explicitly-not-bold run came out . Its sibling MhtmlElementRenderer +# reads the same run correctly, and the two disagreed about the same document. +RSpec.describe Uniword::Transformation::OoxmlToHtmlConverter do + let(:ns) { 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' } + + def run_with(rpr) + Uniword::Wordprocessingml::Run.from_xml( + %(#{rpr}hi), + ) + end + + spellings = { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false }.freeze + + describe ".run_to_html" do + spellings.each do |spelling, bold| + it "#{bold ? 'wraps' : 'does not wrap'} for w:b w:val=#{spelling.inspect}" do + html = described_class.run_to_html(run_with(%())) + + expect(html).to eq(bold ? "hi" : "hi") + end + + it "#{bold ? 'wraps' : 'does not wrap'} for w:i w:val=#{spelling.inspect}" do + html = described_class.run_to_html(run_with(%())) + + expect(html).to eq(bold ? "hi" : "hi") + end + end + + it "wraps a bare , which ST_OnOff reads as on" do + expect(described_class.run_to_html(run_with(""))).to eq("hi") + end + + it "wraps a bare , which ST_OnOff reads as on" do + expect(described_class.run_to_html(run_with(""))).to eq("hi") + end + + it "leaves a run with no toggles alone" do + expect(described_class.run_to_html(run_with(""))).to eq("hi") + end + end + + # The two renderers in this namespace have to agree about the same run. + # They use different tag vocabularies (HTML5 vs HTML4 ), so + # compare whether each emphasised the run at all, not the markup. + describe "agreement with MhtmlElementRenderer" do + spellings.each_key do |spelling| + it "agrees on whether w:val=#{spelling.inspect} is bold" do + run = run_with(%()) + + expect(described_class.run_to_html(run).include?("")) + .to be(Uniword::Transformation::MhtmlElementRenderer.new + .run_to_html(run).include?("")) + end + end + end + + # The bug is reachable from the gem's public API, not just this internal. + describe "through DocumentRoot#to_html_document" do + def document_html(spelling) + Uniword::Wordprocessingml::DocumentRoot.from_xml(<<~XML).to_html_document + + + explicitly not bold + + XML + end + + %w[0 false off].each do |spelling| + it "does not emit for w:b w:val=#{spelling.inspect}" do + expect(document_html(spelling)).not_to include("") + end + end + + %w[1 true on].each do |spelling| + it "emits for w:b w:val=#{spelling.inspect}" do + expect(document_html(spelling)).to include("") + end + end + end +end diff --git a/spec/uniword/transformation/sdt_placeholder_rendering_spec.rb b/spec/uniword/transformation/sdt_placeholder_rendering_spec.rb new file mode 100644 index 00000000..bfad032f --- /dev/null +++ b/spec/uniword/transformation/sdt_placeholder_rendering_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "spec_helper" + +# The renderer used to emit the SDT flags for the mere presence of their +# elements, so came out of the round trip as a +# placeholder Word would show, and as an SDT Word +# would delete on first edit. Both now read the toggle like every other one. +RSpec.describe Uniword::Transformation::MhtmlElementRenderer do + subject(:renderer) { described_class.new } + + let(:ns) { 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' } + + def render(flag) + xml = <<~XML + + + #{flag} + Click here + + + XML + renderer.paragraph_to_html(Uniword::Wordprocessingml::Paragraph.from_xml(xml)) + end + + { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false }.each do |spelling, shown| + it "#{shown ? 'emits' : 'omits'} showingPlcHdr for w:val=#{spelling.inspect}" do + html = render(%()) + + if shown + expect(html).to include('w:showingPlcHdr="t"') + else + expect(html).not_to include("showingPlcHdr") + end + end + end + + it "emits showingPlcHdr for a bare element" do + expect(render("")).to include('w:showingPlcHdr="t"') + end + + it "omits showingPlcHdr when the sdt has no placeholder flag" do + expect(render("")).not_to include("showingPlcHdr") + end + + { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false }.each do |spelling, shown| + it "#{shown ? 'emits' : 'omits'} temporary for w:val=#{spelling.inspect}" do + html = render(%()) + + if shown + expect(html).to include('w:temporary="t"') + else + expect(html).not_to include("temporary") + end + end + end + + it "emits temporary for a bare element" do + expect(render("")).to include('w:temporary="t"') + end + + it "omits temporary when the sdt has no temporary flag" do + expect(render("")).not_to include("temporary") + end +end diff --git a/spec/uniword/wordprocessingml/drawing_alt_text_spec.rb b/spec/uniword/wordprocessingml/drawing_alt_text_spec.rb new file mode 100644 index 00000000..cd3a222c --- /dev/null +++ b/spec/uniword/wordprocessingml/drawing_alt_text_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require "spec_helper" + +# One definition of "what is this image's alt text", shared by the +# accessibility rule, the quality rule, the builder and the renderer. +RSpec.describe Uniword::Wordprocessingml::Drawing do + def drawing_xml(frame: "inline", attrs: "") + <<~XML + + + + + + XML + end + + describe "#alt_text" do + it "reads the docPr descr attribute" do + drawing = described_class.from_xml( + drawing_xml(attrs: %( descr="A leaf on tree bark")), + ) + expect(drawing.alt_text).to eq("A leaf on tree bark") + end + + it "reads descr off an anchored drawing too" do + drawing = described_class.from_xml( + drawing_xml(frame: "anchor", attrs: %( descr="A floating chart")), + ) + expect(drawing.alt_text).to eq("A floating chart") + end + + it "strips surrounding whitespace so padding cannot pass a length check" do + drawing = described_class.from_xml( + drawing_xml(attrs: %( descr=" short ")), + ) + expect(drawing.alt_text).to eq("short") + end + + it "treats a blank descr as absent" do + drawing = described_class.from_xml(drawing_xml(attrs: %( descr=" "))) + expect(drawing.alt_text).to be_nil + end + + it "is nil when the drawing has no descr" do + expect(described_class.from_xml(drawing_xml).alt_text).to be_nil + end + + # ECMA-376 names descr the object's description and title its caption. + # Word's modern Alt Text pane writes descr; the ISO publication corpus + # carries alt text in descr with no title at all. A title is therefore + # not a text alternative, however descriptive it reads. + it "does not fall back to the docPr title" do + drawing = described_class.from_xml( + drawing_xml(attrs: %( title="Photo of a leaf on tree bark")), + ) + + aggregate_failures do + expect(drawing.alt_text).to be_nil + expect(drawing.alt_title).to eq("Photo of a leaf on tree bark") + end + end + end + + describe "#alt_text=" do + it "writes descr onto the drawing's frame" do + drawing = described_class.from_xml(drawing_xml) + drawing.alt_text = "A red square" + + aggregate_failures do + expect(drawing.alt_text).to eq("A red square") + expect(drawing.to_xml).to include('descr="A red square"') + end + end + end +end diff --git a/spec/uniword/wordprocessingml/on_off_attributes_spec.rb b/spec/uniword/wordprocessingml/on_off_attributes_spec.rb new file mode 100644 index 00000000..e89f20db --- /dev/null +++ b/spec/uniword/wordprocessingml/on_off_attributes_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "spec_helper" + +# ST_OnOff also appears as an XML attribute, not just as an element with a +# w:val. Those attributes went through a second reader that raised on any +# token outside the vocabulary, so one malformed attribute anywhere in +# styles.xml killed the parse of the whole part. +RSpec.describe "ST_OnOff attributes" do + ns = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + spellings = { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false, + "banana" => true }.freeze + + describe Uniword::Wordprocessingml::Style do + %w[default customStyle].each do |attr| + spellings.each do |token, expected| + it "reads w:#{attr}=#{token.inspect} as #{expected}" do + style = described_class.from_xml( + %(), + ) + + expect(style.public_send(attr == "default" ? :default : :customStyle)) + .to be(expected) + end + end + end + end + + describe Uniword::Wordprocessingml::LatentStylesException do + { "qFormat" => :q_format, "semiHidden" => :semi_hidden, + "unhideWhenUsed" => :unhide_when_used, "locked" => :locked } + .each do |attr, reader| + spellings.each do |token, expected| + it "reads w:#{attr}=#{token.inspect} as #{expected}" do + exception = described_class.from_xml( + %(), + ) + + expect(exception.public_send(reader)).to be(expected) + end + end + + it "reads an absent w:#{attr} as nil" do + exception = described_class.from_xml( + %(), + ) + + expect(exception.public_send(reader)).to be_nil + end + end + end + + describe Uniword::Wordprocessingml::LatentStyles do + { "defQFormat" => :def_q_format, "defSemiHidden" => :def_semi_hidden, + "defUnhideWhenUsed" => :def_unhide_when_used, + "defLockedState" => :def_locked_state }.each do |attr, reader| + spellings.each do |token, expected| + it "reads w:#{attr}=#{token.inspect} as #{expected}" do + latent = described_class.from_xml( + %(), + ) + + expect(latent.public_send(reader)).to be(expected) + end + end + end + end + + # The point of not raising: a document with one bad token still parses, and + # everything around it survives. + it "parses a styles part that carries a malformed toggle" do + xml = <<~XML + + + + + + + + + XML + + config = nil + expect { config = Uniword::Wordprocessingml::StylesConfiguration.from_xml(xml) } + .not_to raise_error + + aggregate_failures do + expect(config.styles.first.styleId).to eq("Normal") + expect(config.styles.first.name.val).to eq("Normal") + expect(config.styles.first.default).to be(true) + expect(config.latent_styles.count).to eq(2) + expect(config.latent_styles.def_q_format).to be(true) + expect(config.latent_styles.lsd_exception.first.q_format).to be(true) + end + end +end diff --git a/spec/uniword/wordprocessingml/on_off_readers_spec.rb b/spec/uniword/wordprocessingml/on_off_readers_spec.rb new file mode 100644 index 00000000..50dcd0cc --- /dev/null +++ b/spec/uniword/wordprocessingml/on_off_readers_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require "spec_helper" + +# The ST_OnOff toggles that used to sit outside the converged set. +RSpec.describe "ST_OnOff readers outside run properties" do + ns = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + describe Uniword::Wordprocessingml::StructuredDocumentTag::ShowingPlaceholderHeader do + # This element mapped no w:val at all, so an explicitly-off flag came + # back on and the four spellings were indistinguishable. + { "1" => true, "0" => false, "true" => true, "false" => false, + "on" => true, "off" => false }.each do |spelling, expected| + it "reads w:val=#{spelling.inspect} as #{expected}" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + + expect(props.showing_placeholder_header.on?).to be(expected) + end + + it "round-trips w:val=#{spelling.inspect} without flipping" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + reparsed = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(props.to_xml) + + expect(reparsed.showing_placeholder_header.on?).to be(expected) + end + end + + it "reads a bare element as on" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + + expect(props.showing_placeholder_header.on?).to be(true) + end + end + + describe Uniword::Wordprocessingml::StructuredDocumentTag::Temporary do + # Same defect as its showingPlcHdr sibling above: no w:val was mapped, so + # an explicitly-off flag read as on AND the attribute was destroyed on + # write. + { "1" => true, "0" => false, "true" => true, "false" => false, + "on" => true, "off" => false }.each do |spelling, expected| + it "reads w:val=#{spelling.inspect} as #{expected}" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + + expect(props.temporary.on?).to be(expected) + end + + it "round-trips w:val=#{spelling.inspect} without flipping" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + reparsed = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(props.to_xml) + + expect(reparsed.temporary.on?).to be(expected) + end + end + + # The write half: an off flag has to survive as an attribute on disk, not + # come back out as a bare that every reader calls on. + # Assert the value that was written, not merely that some w:val exists — + # w:val="1" would satisfy a bare presence check while inverting the flag. + %w[0 false off].each do |spelling| + it "writes w:val=#{spelling.inspect} back out unchanged" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + + expect(props.to_xml).to match(/]*w:val="#{spelling}"/) + end + end + + it "reads a bare element as on" do + props = Uniword::Wordprocessingml::StructuredDocumentTagProperties + .from_xml(%()) + + expect(props.temporary.on?).to be(true) + end + + # Built in code rather than parsed. This is the half BooleanValSetter + # owns: it normalises an assigned boolean so an on toggle writes the bare + # element Word expects and an off one writes w:val="false". + it "writes a bare element when built on" do + expect(described_class.new(value: true).to_xml).not_to match(/val=/) + end + + # "false" specifically: BooleanValSetter normalises an assigned Ruby + # boolean to the one ST_OnOff spelling, so the output is not merely "some + # off token". + it "writes val=\"false\" when built off" do + expect(described_class.new(value: false).to_xml).to match(/val="false"/) + end + + it "reads back what it built, both ways" do + expect([true, false].map { |v| described_class.new(value: v).on? }) + .to eq([true, false]) + end + end + + describe Uniword::Wordprocessingml::UpdateFields do + { "1" => true, "0" => false, "true" => true, "false" => false, + "on" => true, "off" => false }.each do |spelling, expected| + it "reads w:val=#{spelling.inspect} as #{expected}" do + expect(described_class.from_xml(%()).on?) + .to be(expected) + end + end + + it "reads a bare element as on, the way Word does" do + expect(described_class.from_xml(%()).on?).to be(true) + end + + # The old OoxmlBoolean-typed attribute raised on anything outside the + # ST_OnOff vocabulary. A reader must not blow up on a malformed document. + it "reads an unknown token as on instead of raising" do + expect(described_class.from_xml(%()).on?) + .to be(true) + end + + it "still exposes value as a boolean" do + expect(described_class.new.value).to be(true) + end + end +end diff --git a/spec/uniword/wordprocessingml/paragraph_properties_yaml_spec.rb b/spec/uniword/wordprocessingml/paragraph_properties_yaml_spec.rb new file mode 100644 index 00000000..e3354e8b --- /dev/null +++ b/spec/uniword/wordprocessingml/paragraph_properties_yaml_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require "spec_helper" + +# lutaml-model hands a `to:` transform the accumulating hash and discards what +# the method returns, so a writer that returns its value writes nothing. Every +# pPr writer did that, and a fully populated pPr serialized to "--- {}". +RSpec.describe Uniword::Wordprocessingml::ParagraphProperties do + let(:ns) { 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' } + + def parse(inner) + described_class.from_xml("#{inner}") + end + + describe "#to_yaml" do + it "writes every key a populated pPr sets" do + props = parse(<<~XML) + + + + + + + + + + + XML + + expect(YAML.safe_load(props.to_yaml)).to eq( + "style" => "Heading1", + "alignment" => "center", + "keep_next" => true, + "keep_lines" => true, + "page_break_before" => true, + "widow_control" => true, + "contextual_spacing" => true, + "suppress_line_numbers" => true, + "bidirectional" => true, + "outline_level" => 2, + ) + end + + it "leaves out the keys this pPr does not set" do + expect(YAML.safe_load(parse("").to_yaml)) + .to eq("keep_next" => true) + end + + # w:val="0" and w:val="off" are off. A toggle that is explicitly off has + # to reach YAML as false, not vanish and not come back as true. + { "0" => false, "false" => false, "off" => false, + "1" => true, "true" => true, "on" => true }.each do |spelling, expected| + it "writes the toggles as #{expected} for w:val=#{spelling.inspect}" do + props = parse(<<~XML) + + + + + + + + XML + + expect(YAML.safe_load(props.to_yaml).values.uniq).to eq([expected]) + end + end + end + + # w:suppressLineNumbers and w:bidi are ST_OnOff elements. Declared as plain + # :boolean attributes they read "" for every spelling, so an explicitly-off + # flag was indistinguishable from an on one, and re-serializing dropped the + # element entirely. + describe "#suppress_line_numbers and #bidirectional" do + { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false }.each do |spelling, expected| + it "reads w:val=#{spelling.inspect} as #{expected}" do + props = parse(<<~XML) + + + XML + + aggregate_failures do + expect(props.suppress_line_numbers).to be(expected) + expect(props.bidirectional).to be(expected) + end + end + end + + it "reads a bare element as on" do + props = parse("") + + aggregate_failures do + expect(props.suppress_line_numbers).to be(true) + expect(props.bidirectional).to be(true) + end + end + + it "reads an absent element as off" do + props = parse("") + + aggregate_failures do + expect(props.suppress_line_numbers).to be(false) + expect(props.bidirectional).to be(false) + end + end + + it "keeps the elements over a serialize cycle" do + props = parse(%()) + reparsed = described_class.from_xml(props.to_xml) + + aggregate_failures do + expect(reparsed.suppress_line_numbers).to be(false) + expect(reparsed.bidirectional).to be(true) + expect(props.to_xml).to include("suppressLineNumbers") + expect(props.to_xml).to include("bidi") + end + end + + it "writes a w:val attribute rather than element text" do + props = described_class.new(suppress_line_numbers: false, bidirectional: false) + + aggregate_failures do + expect(props.to_xml).to match(%r{<(w:)?suppressLineNumbers w:val="false"\s*/>}) + expect(props.to_xml).to match(%r{<(w:)?bidi w:val="false"\s*/>}) + end + end + end +end diff --git a/spec/uniword/wordprocessingml/run_properties_yaml_spec.rb b/spec/uniword/wordprocessingml/run_properties_yaml_spec.rb new file mode 100644 index 00000000..4d6cbd7b --- /dev/null +++ b/spec/uniword/wordprocessingml/run_properties_yaml_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require "spec_helper" + +# lutaml-model hands a custom `to:` method the accumulating hash and drops +# its return value, so RunProperties#to_yaml used to emit "--- {}" for a +# fully populated rPr and StyleSet exports silently lost every run property. +RSpec.describe Uniword::Wordprocessingml::RunProperties do + ns = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' + + let(:rpr) do + described_class.from_xml(<<~XML) + + + + + + + + + XML + end + + describe "#to_yaml" do + it "emits the properties the rPr actually carries" do + expect(YAML.safe_load(rpr.to_yaml)).to eq( + "bold" => true, + "italic" => false, + "caps" => true, + "size" => 24, + "color" => "FF0000", + "font" => "Arial", + ) + end + + it "leaves out properties the rPr does not set" do + minimal = described_class.from_xml(%()) + expect(YAML.safe_load(minimal.to_yaml)).to eq("bold" => true) + end + + # A w:val of "0" is off. Emitting it as true was the same defect the + # XML readers had. + it "writes an off toggle as false, not true" do + off = described_class.from_xml(%()) + expect(YAML.safe_load(off.to_yaml)).to eq("bold" => false) + end + end + + describe "#from_yaml" do + it "builds an off toggle from a false value" do + loaded = described_class.from_yaml("bold: false\n") + + aggregate_failures do + expect(loaded.bold?).to be(false) + expect(loaded.to_xml).to include('w:val="false"') + end + end + + it "builds an off toggle from the string \"0\"" do + expect(described_class.from_yaml("bold: \"0\"\n").bold?).to be(false) + end + end + + describe "a YAML round trip" do + it "preserves every property it wrote" do + round_tripped = described_class.from_yaml(rpr.to_yaml) + + aggregate_failures do + expect(round_tripped.bold?).to be(true) + expect(round_tripped.italic?).to be(false) + expect(round_tripped.caps?).to be(true) + expect(round_tripped.size.value).to eq(24) + expect(round_tripped.color.value).to eq("FF0000") + expect(round_tripped.font).to eq("Arial") + end + end + end +end diff --git a/spec/uniword/wordprocessingml/style_yaml_spec.rb b/spec/uniword/wordprocessingml/style_yaml_spec.rb new file mode 100644 index 00000000..9bcbb338 --- /dev/null +++ b/spec/uniword/wordprocessingml/style_yaml_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "spec_helper" + +# lutaml-model hands a custom `to:` method the accumulating hash and drops its +# return value, so every Style writer that returned a value wrote nothing. +# Style#to_yaml silently lost name, quick_format, based_on, next_style, +# linked_style and ui_priority — six keys, one of them an ST_OnOff toggle. +RSpec.describe Uniword::Wordprocessingml::Style do + let(:ns) { 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' } + + def parse(inner, attrs = "") + described_class.from_xml( + %() + + "#{inner}", + ) + end + + describe "#to_yaml" do + it "writes every key a populated style sets" do + style = parse(<<~XML) + + + + + + + XML + + expect(YAML.safe_load(style.to_yaml)).to eq( + "id" => "Heading1", + "type" => "paragraph", + "name" => "heading 1", + "quick_format" => true, + "based_on" => "Normal", + "next_style" => "Normal", + "linked_style" => "Heading1Char", + "ui_priority" => 9, + ) + end + + it "leaves out the keys the style does not set" do + expect(YAML.safe_load(parse("").to_yaml).keys).to eq(%w[id type]) + end + + # Only the six keys these writers own. w:default and w:customStyle are + # plain `map` entries that read back as false rather than absent; that + # asymmetry predates this change and is not what these writers control. + # Both sides are compared against an explicit hash, not against each other: + # if the six writers regressed, both serializations would lose the same six + # keys and a self-comparison would still pass. + it "survives the YAML round trip" do + keys = %w[name quick_format based_on next_style linked_style ui_priority] + expected = { + "name" => "heading 1", + "quick_format" => false, + "based_on" => "Normal", + "next_style" => "Normal", + "linked_style" => "Heading1Char", + "ui_priority" => 9, + } + style = parse(<<~XML) + + + + + + + XML + + reparsed = described_class.from_yaml(style.to_yaml) + + expect(YAML.safe_load(style.to_yaml).slice(*keys)).to eq(expected) + expect(YAML.safe_load(reparsed.to_yaml).slice(*keys)).to eq(expected) + end + end + + # w:qFormat is ST_OnOff. Every spelling has to reach YAML as the boolean + # Word would show, not as the mere presence of the element. + describe "#to_yaml quick_format across the ST_OnOff spellings" do + { "1" => true, "true" => true, "on" => true, + "0" => false, "false" => false, "off" => false }.each do |spelling, expected| + it "writes quick_format #{expected} for w:val=#{spelling.inspect}" do + style = parse(%()) + + expect(YAML.safe_load(style.to_yaml)["quick_format"]).to be(expected) + end + end + + it "writes quick_format true for a bare element" do + expect(YAML.safe_load(parse("").to_yaml)["quick_format"]).to be(true) + end + + it "leaves quick_format out when the style has no w:qFormat" do + expect(YAML.safe_load(parse("").to_yaml)).not_to have_key("quick_format") + end + end +end diff --git a/spec/uniword/wordprocessingml/styles_spec.rb b/spec/uniword/wordprocessingml/styles_spec.rb index 12a70dfa..1475c361 100644 --- a/spec/uniword/wordprocessingml/styles_spec.rb +++ b/spec/uniword/wordprocessingml/styles_spec.rb @@ -47,6 +47,81 @@ expect(style.paragraph_style?).to be false end end + + # Every ST_OnOff reader on Style routes through the private boolean_flag + # helper, which delegates to Properties::BooleanElement#on?. The full + # lexical table lives in spec/uniword/properties/bold_spec.rb; these only + # prove each reader delegates rather than hand-rolling the decision. + describe "ST_OnOff readers" do + def style_with(body) + wml = "http://schemas.openxmlformats.org/wordprocessingml/2006/main" + described_class.from_xml(<<~XML) + + #{body} + + XML + end + + describe "#bold and #italic" do + it "reads an off token as false" do + style = style_with(%()) + + expect(style.bold).to be(false) + expect(style.italic).to be(false) + end + + it "reads an on token as true" do + style = style_with(%()) + + expect(style.bold).to be(true) + expect(style.italic).to be(true) + end + + it "returns nil when the toggle element is absent" do + style = style_with("") + + expect(style.bold).to be_nil + expect(style.italic).to be_nil + end + end + + describe "#quick_format" do + it "reads an off token as false" do + expect(style_with(%()).quick_format).to be(false) + end + + it "reads a bare element as true" do + expect(style_with("").quick_format).to be(true) + end + + it "is false when absent" do + expect(style_with("").quick_format).to be(false) + end + end + + describe "#keep_next and #keep_lines" do + it "reads off tokens as false" do + style = style_with( + %(), + ) + + expect(style.keep_next).to be(false) + expect(style.keep_lines).to be(false) + end + + it "reads bare elements as true" do + style = style_with(%()) + + expect(style.keep_next).to be(true) + expect(style.keep_lines).to be(true) + end + + it "is false when pPr is absent" do + expect(style_with("").keep_next).to be(false) + expect(style_with("").keep_lines).to be(false) + end + end + end end RSpec.describe Uniword::Wordprocessingml::StylesConfiguration do diff --git a/spec/uniword/wordprocessingml/update_fields_spec.rb b/spec/uniword/wordprocessingml/update_fields_spec.rb index 78fcdf14..8d1598e9 100644 --- a/spec/uniword/wordprocessingml/update_fields_spec.rb +++ b/spec/uniword/wordprocessingml/update_fields_spec.rb @@ -15,8 +15,16 @@ expect(xml).to include("updateFields") end + # w:updateFields now goes through the same ST_OnOff element machinery as + # every other toggle, which spells off as w:val="false". The assertion is + # about the meaning, not the spelling: both are ST_OnOff off. it "serializes w:val when false" do - expect(described_class.new(value: false).to_xml).to include('w:val="0"') + xml = described_class.new(value: false).to_xml + + aggregate_failures do + expect(xml).to match(/w:val="(0|false|off)"/) + expect(described_class.from_xml(xml).on?).to be(false) + end end it "parses w:updateFields from settings XML" do