Closed
Bug 842142
Opened 11 years ago
Closed 11 years ago
Text incorrectly wraps with svg.text.css-frames.enabled
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: longsonr, Assigned: heycam)
References
()
Details
Attachments
(3 files, 3 obsolete files)
banana should follow "You are not a" on the same line but wraps instead. Same with "But you" on right hand side.
Reporter | ||
Updated•11 years ago
|
Assignee | ||
Comment 1•11 years ago
|
||
The wrapping seems to be OK on Mac. I do however get wrapping where I shouldn't with this attached case if I full page zoom it in one step.
Assignee | ||
Comment 2•11 years ago
|
||
I did the get wrapping in that test once. It seems quite intermittent.
Assignee | ||
Comment 3•11 years ago
|
||
There is some timing dependence on this bug manifesting. On my machine, and with my network characteristics, I can reliably reproduce with this reduced test: http://mcc.id.au/temp/tspan.html The SVG is being loaded inside an <object>. If I change this to an <iframe>, the bug is gone. It looks like the <object> is sized initially so that the root SVG element has mViewport{Width,Height} = 5. Then later it gets resized to 480x360. The small viewport results in a large mFontSizeScaleFactor, which in turn is going to result in slightly different text layout from when it is at 480x360. I think we're not reflowing the text once the <object> gets its final size and the viewport changes.
Assignee | ||
Comment 4•11 years ago
|
||
This patch fixes the problem, but at the expense of reflowing the text whenever the transform changes. It would be better if we could avoid reflowing if it's not a change in the scale. Unfortunately in NotifySVGChange when (aFlags & TRANSFORM_CHANGED), we don't have access to the old transform. Ultimately the reason why we get these word wraps is that we reflow the anonymous block frame at the exact width it reports as being its preferred width. When the metrics change slightly without reflowing, it can end up wrapping. The change in AttributeChanged is so that we actually invalidate if our own transform changes. NotifySVGChange(TRANSFORM_CHANGED) won't invalidate us, since it handles the case when an ancestor's transform changed, and we would be included in their invalidation area already.
Assignee | ||
Comment 5•11 years ago
|
||
This is the right patch.
Attachment #715302 -
Attachment is obsolete: true
Attachment #715302 -
Flags: review?(longsonr)
Attachment #715304 -
Flags: review?(longsonr)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 715304 [details] [diff] [review] patch With this patch applied, if I view http://www.w3.org/Graphics/SVG/Test/20110816/svg/text-text-03-b.svg and full page zoom it, the page flashes each time I zoom, as it is briefly not rendering the text. We'll need a different fix.
Attachment #715304 -
Flags: review?(longsonr)
Assignee | ||
Comment 7•11 years ago
|
||
From the comments in bug 839865, it looks like calling InvalidateBounds() from AttributeChanged is wrong anyway.
Reporter | ||
Comment 8•11 years ago
|
||
Surely it's better to either a) make the outer svg element default to whitespace: nowrap, or b) fix nsTextFrameThebes such that anywhere it calls WhiteSpaceCanWrap() or WordCanWrap() we also check for IsSVGText() and return false.
Assignee | ||
Comment 9•11 years ago
|
||
I like option (b).
Assignee | ||
Comment 10•11 years ago
|
||
This makes WhiteSpaceCanWrap and WordCanWrap sensitive to whether the frame is for SVG text, and returns false for both. This solves the unexpected text wrapping problem, although there is a related problem where the SVG text inside the <object> is not selectable initially. So we still probably need to reflow the text due to the <object> resizing, but I'll look at that in a separate bug.
Attachment #715304 -
Attachment is obsolete: true
Attachment #728811 -
Flags: review?(roc)
Assignee | ||
Comment 12•11 years ago
|
||
The actual patch.
Attachment #728811 -
Attachment is obsolete: true
Attachment #728811 -
Flags: review?(roc)
Attachment #728817 -
Flags: review?(roc)
Attachment #728817 -
Flags: review?(roc) → review+
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a715855b639
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #10) > So we still probably > need to reflow the text due to the <object> resizing, but I'll look at that > in a separate bug. Bug 854286.
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6a715855b639
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•