Last Comment Bug 414112 - CSS 2.1 300px fallback width doesn't play nice with percentages other than 100%
: CSS 2.1 300px fallback width doesn't play nice with percentages other than 100%
Status: VERIFIED FIXED
: testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: ---
Assigned To: Jonathan Watt [:jwatt]
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-26 02:33 PST by Jonathan Watt [:jwatt]
Modified: 2008-05-08 13:03 PDT (History)
7 users (show)
mtschrep: blocking1.9+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.26 KB, application/xhtml+xml)
2008-01-26 02:33 PST, Jonathan Watt [:jwatt]
no flags Details
patch-ish (2.50 KB, patch)
2008-01-26 07:35 PST, Jonathan Watt [:jwatt]
no flags Details | Diff | Splinter Review
patch (1.29 KB, patch)
2008-02-05 06:26 PST, Jonathan Watt [:jwatt]
bzbarsky: review+
tor: superreview+
Details | Diff | Splinter Review

Description Jonathan Watt [:jwatt] 2008-01-26 02:33:19 PST
Created attachment 299401 [details]
testcase

Whenever we have <svg width="X%"> inside a containing block (CB) that depends on the <svg>'s width, the <svg> should fall back to a width of 300px. Instead, the <svg> falls back to a width of X% of 300px.

This bug is caused because the layout code sizes the CB by calling GetPrefWidth on it's children. When the <svg> returns 300px in the testcase, the CB takes that width, and this is the width passed into nsLayoutUtils::ComputeSizeWithIntrinsicDimensions as aCBSize.width. Unfortunately the intrinsic width of X% is then applied to aCBSize.width to calculate the <svg>'s used width, and hence the bug.

I'm not really sure how best to fix this. We can't pre-multiply the value nsSVGOuterSVGFrame::GetPrefWidth returns in preparation for ComputeSizeWithIntrinsicDimensions, since then the CB would take the wrong size. Perhaps there's a way to tell in ComputeSizeWithIntrinsicDimensions whether the CB's width depends on us, but we only get given the CB's size, not the CB, so I don't see how.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2008-01-26 07:05:52 PST
> Whenever we have <svg width="X%"> inside a containing block (CB) that depends
> on the <svg>'s width, the <svg> should fall back to a width of 300px.

I assume the issue is this spec text?

  Percentage intrinsic widths are first evaluated with respect to the
  containing block's width, if that width doesn't itself depend on the
  replaced element's width. If it does, then a percentage intrinsic
  width on that element can't be resolved and the element is assumed
  to have no intrinsic width. 

Telling whether the CB width depends on us is actually pretty nontrivial...   It might make more sense to change the spec here or something.

I'll post to www-style about this....
Comment 2 Jonathan Watt [:jwatt] 2008-01-26 07:33:14 PST
That's correct.
Comment 3 Jonathan Watt [:jwatt] 2008-01-26 07:35:48 PST
Created attachment 299419 [details] [diff] [review]
patch-ish

So I guess something like this doesn't cover all the bases? (It fixes the testcase.)
Comment 4 Jonathan Watt [:jwatt] 2008-01-26 16:55:42 PST
Come to think about it, we can probably set a bit on the nsSVGOuterSVGFrame in GetPrefWidth so we know further on in reflow that our CB's width depends on us. We'd need to clear the bit in MarkIntrinsicWidthsDirty since GetPrefWidth can be called for reasons other than reflow, but this trick should allow us to cover all the bases I think. Not elegant, but effective. Thoughts?
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2008-01-27 10:24:40 PST
> So I guess something like this doesn't cover all the bases?

It'll treat a basic <div> as depending on the width of the SVG, whereas in reality it does not in this testcase:

  <html><div><svg/></div></html>

As for comment 4, there's no guarantee that GetPrefWidth() being called means that the ancestor in fact depends on your width.
Comment 6 Jonathan Watt [:jwatt] 2008-02-05 06:14:44 PST
Comment on attachment 299419 [details] [diff] [review]
patch-ish

Actually this breaks a whole bunch of the SVG sizing reftests.
Comment 7 Jonathan Watt [:jwatt] 2008-02-05 06:23:01 PST
So Boris convinced me and www-style that the 300px thing doesn't work. That section has been removed from the CSS 2.1 draft: http://lists.w3.org/Archives/Public/www-style/2008Feb/0029.html

I guess percentage intrinsic widths on elements in a CB with dependent width should resolve against the width the CB would have if all the CB's children with percentage widths were given a width of zero. In other words nsSVGOuterSVGFrame::GetPrefWidth should return zero.
Comment 8 Jonathan Watt [:jwatt] 2008-02-05 06:26:42 PST
Created attachment 301493 [details] [diff] [review]
patch
Comment 9 Jonathan Watt [:jwatt] 2008-02-08 13:50:44 PST
Checked in.
Comment 10 Jonathan Watt [:jwatt] 2008-02-08 13:52:21 PST
This bug reverted the change introduced in bug 382325 BTW.
Comment 11 Al Billings [:abillings] 2008-05-07 15:39:44 PDT
I just looked at this using the testcase in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050706 Minefield/3.0pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre and I'm getting a blank window with this test case. 

I suspect that another bug is masking this but if I look the testcase in current branch, I get a red square and a blue square drawn on the screen (which is better than nothing at all).
Comment 12 Jonathan Watt [:jwatt] 2008-05-07 16:00:15 PDT
A blank window is exactly what's expected. The floated div has no explicit width or height, so its dimensions depend on its content. If it has no content, or if its content takes up no room, the width and height of the div will be zero. As it happens the div does have some content, but this content has a percentage width and height. These percentages need to be resolved, but how do you resolve them with the cyclic dependency of the parent div depending on its content? We decided that the best thing we could do was to resolve the percentages against the dimensions that the parent div would have if it didn't include the children with the percentage dimensions. In other words the percentages in this case are resolved as percentages of zero. I.e., zero.
Comment 13 Jonathan Watt [:jwatt] 2008-05-07 16:02:10 PDT
Oh, and this comes down to Boris saying it's pretty much impossible in general to know when the containing block's dimensions depend on us, so it's not possible to implement the 300 x 150 px fallback. If I remember correctly, Boris brought that up on www-style, and that section has now been removed from CSS 2.1 draft.
Comment 14 Al Billings [:abillings] 2008-05-07 16:11:11 PDT
All right. Thanks, Jonathan. I'll mark this as verified then. 

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre
Comment 15 Jeff Walden [:Waldo] (remove +bmo to email) 2008-05-08 13:03:14 PDT
Could we add the test as a reftest, then?

Note You need to log in before you can comment on or make changes to this bug.