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

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bugzilla, Assigned: smontagu)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla41
testcase
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
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.
(Reporter)

Updated

4 years ago
Blocks: 145503
Keywords: testcase
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.
This is presumably a result of the recent landing of bug 1147834 and bug 1079151.
(Assignee)

Comment 3

4 years ago
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.
Assignee: nobody → smontagu
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1172777
(Assignee)

Comment 5

4 years ago
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?
Flags: needinfo?(bugzilla)
(Reporter)

Comment 6

4 years ago
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.
(Reporter)

Updated

4 years ago
Flags: needinfo?(bugzilla)
(Assignee)

Comment 7

4 years ago
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.
(Reporter)

Comment 8

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

Comment 10

4 years ago
Created attachment 8617935 [details] [diff] [review]
Patch

This also picks up a callsite of the nsCSSOffsetState constructor in nsBlockFrame.cpp which was passing in width instead of inline-size.
Attachment #8617935 - Flags: review?(jfkthame)
(Assignee)

Comment 11

4 years ago
Created attachment 8617950 [details] [diff] [review]
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?)
Attachment #8617950 - Flags: review?(jfkthame)
Attachment #8617935 - Flags: review?(jfkthame) → review+
Attachment #8617950 - Flags: review?(jfkthame) → review+
https://hg.mozilla.org/mozilla-central/rev/bcaca4f837e8
https://hg.mozilla.org/mozilla-central/rev/6fd48f8bf7b4
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(Reporter)

Comment 14

4 years ago
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.
(Reporter)

Comment 15

4 years ago
> 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
You need to log in before you can comment on or make changes to this bug.