CSS 2.1 300px fallback width doesn't play nice with percentages other than 100%

VERIFIED FIXED

Status

()

Core
SVG
P2
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

({testcase})

Trunk
testcase
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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.
> 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....
(Assignee)

Comment 2

10 years ago
That's correct.
(Assignee)

Comment 3

10 years ago
Created attachment 299419 [details] [diff] [review]
patch-ish

So I guess something like this doesn't cover all the bases? (It fixes the testcase.)
(Assignee)

Comment 4

10 years ago
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?
> 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.
Flags: blocking1.9?
(Assignee)

Comment 6

10 years ago
Comment on attachment 299419 [details] [diff] [review]
patch-ish

Actually this breaks a whole bunch of the SVG sizing reftests.
Attachment #299419 - Attachment is obsolete: true
(Assignee)

Comment 7

10 years ago
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.
(Assignee)

Comment 8

10 years ago
Created attachment 301493 [details] [diff] [review]
patch
Assignee: nobody → jwatt
Status: NEW → ASSIGNED
Attachment #301493 - Flags: superreview?(tor)
Attachment #301493 - Flags: review?(bzbarsky)
Attachment #301493 - Flags: review?(bzbarsky) → review+

Updated

10 years ago
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2

Updated

10 years ago
Attachment #301493 - Flags: superreview?(tor) → superreview+
(Assignee)

Comment 9

10 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

10 years ago
This bug reverted the change introduced in bug 382325 BTW.
(Assignee)

Updated

10 years ago
Summary: CSS 2.1 300px fallback width doesn't play nice with percentages → CSS 2.1 300px fallback width doesn't play nice with percentages other than 100%
Flags: in-testsuite?
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).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

9 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

9 years ago
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.
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
Status: RESOLVED → VERIFIED
Could we add the test as a reftest, then?
You need to log in before you can comment on or make changes to this bug.