Closed Bug 617737 Opened 9 years ago Closed 9 years ago

SVG text white space handling incorrect

Categories

(Core :: SVG, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: heycam, Assigned: longsonr)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached image Reduced test
Sometimes white space characters are rendered in SVG text when they shouldn't be.  This seems to be when white space occurs at the end of a text node, the text node has a following element sibling, yet the white space is at the end of the whole run of text, and so should be trimmed.

This makes Firefox fail http://dev.w3.org/SVG/profiles/1.1F2/test/svg/animate-elem-77-t.svg
Version: unspecified → Trunk
Should this bug block bug 512501? (or, if this testcase is new in SVG 1.1F2, perhaps we need a new tracking bug for issues in the SVG 1.1F2 testsuite?)
The test isn't very new, but it might well be the case that it hasn't been in a published version of the test suite before.

Is it worth tracking 1.1F2 test suite bugs separately?  I think the new test suite release (which will be soon) is effectively an iteration on the previous one, so we should always be looking at the most recent release.

I will have a more up-to-date list of which tests from the to-be-released test suite are failing after today.
Ok - it sounds like it's reasonable to use bug 512501 to track issues with tests in 1.1F2's test suite, then. Marking as blocking that bug.
Blocks: svg11tests
This seems tightly releted with bug 503075 (might even be a duplicate, although I haven't dig down to the details).
Assignee: nobody → longsonr
It's unrelated to bug 503075 despite looking so at first glance.
Attached patch patch (obsolete) — Splinter Review
Attachment #497081 - Attachment is patch: true
Attachment #497081 - Flags: review?(roc)
Comment on attachment 497081 [details] [diff] [review]
patch

Actually ought to implement a HasText method for speed.
Attachment #497081 - Flags: review?(roc)
Comment on attachment 497081 [details] [diff] [review]
patch

Does this work for the case like <tspan>ABC <!--text node boundary--> </tspan> where there is a trailing text node containing only whitespace? It seems to me that if whitespace changes from preserve to trim-trailing, the call to next->GetNumberOfChars() when 'next' is the second text node will return 1 because the correct whitespace-handling flag has not been set yet.

It seems to me you'll have to iterate backwards from the last text node somewhow.
Attached patch updated patch (obsolete) — Splinter Review
Attachment #497081 - Attachment is obsolete: true
Attachment #497131 - Flags: review?(roc)
Comment on attachment 497131 [details] [diff] [review]
updated patch

+    if (NS_IsAsciiWhitespace(str[i]))
+      continue;
+    return PR_FALSE;

if (!...)
  return PR_FALSE;
Attachment #497131 - Flags: review?(roc) → review+
Attachment #497131 - Attachment is obsolete: true
Comment on attachment 497390 [details] [diff] [review]
hg changeset patch

Addressed review comments.

This fixes one of the SVG testsuite tests and includes a reftest.
Attachment #497390 - Flags: approval2.0?
Keywords: checkin-needed
Whiteboard: [needs landing]
Keywords: checkin-needed
Whiteboard: [needs landing]
pushed http://hg.mozilla.org/mozilla-central/rev/1ae27e2eb959
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.