Closed
Bug 328829
Opened 19 years ago
Closed 18 years ago
style changes within SVG don't propagate to foreignObject correctly
Categories
(Core :: SVG, defect, P2)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Whiteboard: [reflow-refactor][patch])
Attachments
(3 files, 2 obsolete files)
(deleted),
application/xhtml+xml
|
Details | |
(deleted),
application/xhtml+xml
|
Details | |
(deleted),
patch
|
jwatt
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
In bug 328550 I added a comment explaining why style changes requiring reflow that happen within SVG content don't propagate to foreignObject elements correctly. I should write a testcase for this and attach a patch (it's not too hard, although it could slow down SVG reflow a little bit).
Assignee | ||
Updated•19 years ago
|
Summary: style changes within SVG don't propage to foreignObject correctly → style changes within SVG don't propagate to foreignObject correctly
Comment 1•18 years ago
|
||
David, could you take a minute to elaborate on how this should be fixed, or even whip up that patch if you can find the time? :-)
Assignee | ||
Comment 2•18 years ago
|
||
roc made some changes that helped, and the reflow branch should help a little more. Ask me again after it lands.
Whiteboard: [reflow-refactor]
Comment 3•18 years ago
|
||
So the reflow branch has landed. Can I ask again yet David? :-)
Assignee | ||
Updated•18 years ago
|
Flags: blocking1.9?
Assignee | ||
Comment 4•18 years ago
|
||
Here's a testcase, although I'm seeing some refresh problems so nothing at all shows up until I do a zoom-in and zoom-out.
Assignee | ||
Comment 5•18 years ago
|
||
The bug is also present for style changes outside the SVG (although I'm not sure if that was the case when I filed it).
Assignee | ||
Comment 6•18 years ago
|
||
Here's a patch. It's a little hacky, but it should work. And it fixes both problems.
Basically, if a style change requires reflow, we call MarkIntrinsicWidthsDirty on all descendants of the thing with the style change, plus ancestors up to a reflow root. So when a foreignObject gets MarkIntrinsicWidthsDirty should match exactly the cases when it needs reflow due to style changes. And as far as I know, those are the only times when it needs an additional reflow.
Attachment #252834 -
Flags: superreview?(roc)
Attachment #252834 -
Flags: review?(jwatt)
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [reflow-refactor] → [reflow-refactor][patch]
Target Milestone: --- → mozilla1.9beta
Assignee | ||
Comment 7•18 years ago
|
||
Er, except I don't need to use eStyleChange because we're in the middle of MarkIntrinsicWidthsDirty...
Assignee | ||
Updated•18 years ago
|
Attachment #252834 -
Flags: superreview?(roc)
Attachment #252834 -
Flags: review?(jwatt)
Assignee | ||
Comment 8•18 years ago
|
||
Attachment #252834 -
Attachment is obsolete: true
Attachment #252836 -
Flags: superreview?(roc)
Attachment #252836 -
Flags: review?(jwatt)
Comment 9•18 years ago
|
||
Hmm, weird. Both testcases seem to work for me in the nightly spun on the day this bug was filed:
http://archive.mozilla.org/pub/firefox/nightly/2006-02-28-05-trunk/firefox-1.6a1.en-US.win32.zip
(and the build from two days before.) Is that possible? I realized after mistakenly testing the patch in the wrong (unpatched) build which caused me to look back through the archives to see when it had been fixed. Perhaps there are cases this patch would fix that are not tested by the testcases?
Assignee | ||
Comment 10•18 years ago
|
||
For what it's worth, for both testcases:
Expected results: "hello world" entirely visible within page, no overlap
between words
Actual results: "hello" and "world" overlap, and "hello" partly off screen
Comment 11•18 years ago
|
||
Comment on attachment 252836 [details] [diff] [review]
patch
My apologies, you're right of course. The patch does fix the overlap problem. *blush* Strange that the actual text resizing bit works without it. So I don't know very much about layout, but from what you've explained this looks got to me. r=jwatt
Attachment #252836 -
Flags: review?(jwatt) → review+
Assignee | ||
Comment 12•18 years ago
|
||
The bug showed up that way because painting uses the correct style to paint, but if there hasn't been a reflow, it paints in the wrong place.
Comment 13•18 years ago
|
||
Even with this patch and the patch from bug 368683 this testcase fails. The text in the foreignObject continues to wrap at the same width when the SVG is resized.
Comment 14•18 years ago
|
||
Comment on attachment 253332 [details]
testcase 3
Ah, it seems if I use eStyleChange instead of eResize it works. I'll take that testcase to the other bug.
Attachment #253332 -
Attachment is obsolete: true
Attachment #252836 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 15•18 years ago
|
||
Fix checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•18 years ago
|
||
Note that I had to comment out one of the reftests which fails intermittently (bug 369046).
Updated•18 years ago
|
Flags: blocking1.9?
You need to log in
before you can comment on or make changes to this bug.
Description
•