1172774 - Percentage margin applied on block in orthogonal flow (vertical writing-mode)

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Gérard Talbot]

Tests with vendor-prefixes
--------------------------


http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vrl-002.xht


http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vlr-003.xht


Expected result
---------------

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-margin-vrl-002-ref.xht


Explanation
-----------

"
The percentage is calculated with respect to the width of the generated box's containing block. Note that this is true for 'margin-top' and 'margin-bottom' as well.
"
8.3 Margin properties
http://www.w3.org/TR/CSS21/box.html#margin-properties

 div.foo
    {
      margin-bottom: 2.5%; /* 5px */
      margin-left: 50%; /* 100px */
      margin-right: 25%; /* 50px */
      margin-top: 10%; /* 20px */
      ...
    }

but the web inspector tool reports huge, weird numbers like:

margin-bottom: 447392px;
margin-left: 8947850px;
margin-right: 4473920px;
margin-top: 1789570px;


Notes
-----

- Those 2 tests used to pass in Firefox 38.0a1 buildID 20150204150323: see
http://lists.w3.org/Archives/Public/public-css-testsuite/2015Feb/0002.html
- These 2 tests have been submitted to the test.csswg.org repository 
- Chrome 40+ pass these 2 tests; according to my records, Prince 9.0.5 was passing the vertical-rl test (s72-percent-margin-vrl-002) although now Prince 10.2r1 is not
- I am using Firefox 41.0a1 buildID=20150608030201
- I use Linux 3.13.0-53-generic x86_64, Qt: 4.8.6, KDE 4.13.3; Kubuntu (trusty) 14.04.02 LTS
- I've searched for duplicates and did not find any.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

The numbers (as well as those in bug 1172777) indicates that we add NS_UNCONSTRAINEDSIZE with something and calculate the values against it.

The numbers:
margin-bottom: 447392px = (nscoord_MAX - 1024) / 60 * 2.5%
margin-left:  8947850px = (nscoord_MAX + 176 ) / 60 * 50%
margin-right: 4473920px = (nscoord_MAX - 1024) / 60 * 25%
margin-top:   1789570px = (nscoord_MAX + 176 ) / 60 * 10%

It seems we add 176 for {I,B}Start side and minus 1024 for {I,B}End side.

Comment 2

[Jonathan Kew [:jfkthame]]

This is presumably a result of the recent landing of bug 1147834 and bug 1079151.

Comment 3

[Simon Montagu :smontagu]

This seems to be attributable to one of the changes in attachment 8609999 [details] [diff] [review] -- shame that I didn't have the courage of my convictions that my original patch was correct. OffsetPercentBasis should be using the containing block's size *in its own writing mode*, not in the current writing mode -- see http://dev.w3.org/csswg/css-writing-modes-3/#orthogonal-flows

"In the positioning phase—calculating the positioning offsets, margins, borders, and padding—the dimensions of the box and its containing block are mapped to the inline size and block size and calculations are performed according to the writing mode of the containing block of the box establishing the orthogonal flow."

However, it's not just a one line fix, because that causes a lot of "writing-mode mismatch" assertions, so a little more work is needed.

Comment 5

[Simon Montagu :smontagu]

Gérard, do you already have tests for the opposite case, i.e. where the outer div is in a vertical writing mode and the inner div horizontal?

Comment 6

[Gérard Talbot]

Simon, no I do not have tests for the opposite case. 

Now that you are asking me such question, I realize I should have done tests for the opposite case in the first place. I am therefore adding this into my to-do-list plan; I have a busy schedule this week and am not sure exactly when I will do such tests.

Comment 7

[Simon Montagu :smontagu]

That's no problem, I'll add a flipped version of your tests as reftests for this bug. I just didn't want to duplicate work if they were already around.

Comment 8

[Gérard Talbot]

Simon: a quick (very late!) test where the outer div is in vertical-rl writing mode and the inner divs are in horizontal-tb writing-mode:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vrl-006.xht

and its reference file:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-margin-vrl-002-ref.xht

Firefox 41.0a1 buildID=20150513191426 passes this test.


More later this week...

Comment 9

[Simon Montagu :smontagu]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d2ff7f3b7ad

Comment 10

[Simon Montagu :smontagu]

Attachment: Patch

This also picks up a callsite of the nsCSSOffsetState constructor in nsBlockFrame.cpp which was passing in width instead of inline-size.

Comment 11

[Simon Montagu :smontagu]

Attachment: Reftests

These are Gérard's tests for margin from here and padding from bug 1172777, split into test and reftest, plus versions with horizontal writing mode within vertical (with the reference image rotated 90° instead of the same like the test in comment 8, since I didn't take as much trouble to make the results come out the same).

I suppose it would be good to add tests for relative offsets since the patch affects them as well, but maybe for positioning, unlike padding and margins, the calculation is symmetric so there isn't a case where it makes a difference? (thinking aloud here -- WDYS?)

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bcaca4f837e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fd48f8bf7b4

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bcaca4f837e8
https://hg.mozilla.org/mozilla-central/rev/6fd48f8bf7b4

Comment 14

[Gérard Talbot]

Tests with vendor-prefixes
--------------------------

Margin
- - - 

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-rl':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vrl-002.xht

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vlr-003.xht

containing block's writing-mode and children's writing-mode is 'vertical-rl':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vrl-004.xht

containing block's writing-mode and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vlr-005.xht

containing block's writing-mode is 'vertical-rl' and children's writing-mode is 'horizontal-tb':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vrl-006.xht

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vlr-007.xht


-----


Padding
- - - -

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-rl':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vrl-002.xht

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vlr-003.xht

containing block's writing-mode and children's writing-mode is 'vertical-rl':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vrl-004.xht

containing block's writing-mode and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vlr-005.xht

containing block's writing-mode is 'vertical-rl' and children's writing-mode is 'horizontal-tb':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vrl-006.xht

containing block's writing-mode is 'horizontal-tb' and children's writing-mode is 'vertical-lr':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vlr-007.xht



Expected result for each and all 12 tests
-----------------------------------------

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-margin-vrl-002-ref.xht


These tests have been (8) or will be submitted (4) to the CSS3 Writing-Modes test suite.

Comment 15

[Gérard Talbot]

> containing block's writing-mode is 'horizontal-tb' and children's
> writing-mode is 'vertical-lr':
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-
> margin-vlr-007.xht

Correction:

containing block's writing-mode is 'vertical-lr' and children's writing-mode is 'horizontal-tb':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-margin-vlr-007.xht


> containing block's writing-mode is 'horizontal-tb' and children's
> writing-mode is 'vertical-lr':
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-
> padding-vlr-007.xht

Correction:

containing block's writing-mode is 'vertical-lr' and children's writing-mode is 'horizontal-tb':
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s72-percent-padding-vlr-007.xht