Closed Bug 1079151 Opened 6 years ago Closed 5 years ago

Logical coordinate support for absolute positioning

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: smontagu, Assigned: jfkthame)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(8 files, 11 obsolete files)

515.84 KB, patch
jfkthame
: review+
Details | Diff | Splinter Review
20.12 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
6.52 KB, text/html
Details
299.00 KB, patch
Details | Diff | Splinter Review
47.35 KB, patch
Details | Diff | Splinter Review
1.56 KB, text/html
Details
18.88 KB, patch
Details | Diff | Splinter Review
9.66 KB, patch
Details | Diff | Splinter Review
Part of support for vertical text
Early this month, I submitted 96 abs-pos-non-replaced tests
http://lists.w3.org/Archives/Public/public-css-testsuite/2015Mar/0000.html
to the Writing-modes test suite.

Those 96 tests cover/are the combinations of the following code scenarios:

2 writing-modes [vertical-rl | vertical-lr]
3 containing-block contexts (not orthogonal, orthogonal and then orthogonal on abs-pos child)
2 directions [rtl | ltr] for containing-block
8 'auto' combinations (2^3 combinations)
  {
  If all three of 'left', 'width', and 'right' are 'auto'
  6 other 'auto' combinations
  If none of the three is 'auto'
  }
===============
96 combinations

Out of 96 tests, Firefox 39.0a1 buildID=20150306184824 pass 22 tests (23%) and fail 74 tests (77%). In comparison, IE11 fails about 40% (I do not know the exact number right now) and Chrome 40+ fails 24 tests (25%).

Those 96 tests have not been reviewed yet. Here are those 96 tests with vendor-prefixes:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-002.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-003.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-004.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-005.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-006.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-007.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-008.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-009.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-010.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-011.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-012.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-013.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-014.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-015.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-016.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-017.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-018.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-019.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-020.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-021.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-022.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-023.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-024.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-025.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-026.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-027.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-028.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-029.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-030.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-031.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-032.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-033.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-034.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-035.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-036.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-037.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-038.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-039.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-040.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-041.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-042.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-043.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-044.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-045.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-046.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-047.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-048.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-049.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-050.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-051.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-052.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-053.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-054.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-055.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-056.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-057.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-058.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-059.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-060.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-061.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-062.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-063.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-064.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-065.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-066.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-067.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-068.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-069.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-070.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-071.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-072.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-073.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-074.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-075.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-076.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-077.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-078.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-079.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-080.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-081.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-082.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-083.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-084.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-085.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-086.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-087.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-088.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-089.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-090.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-091.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-092.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-093.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-094.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-095.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-096.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-097.xht


The most noticeable failed (by Firefox 39) tests are:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-064.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-065.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-070.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-071.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-088.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-089.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-094.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-095.xht

Gérard
Keywords: testcase
Version: unspecified → Trunk
Blocks: 1158653
Here are a bunch of examples that set position:absolute but give no explicit offsets, so the positioned element should stay in its original position. We currently fail all the vertical writing-mode examples here.

For comparison, Chrome renders this as expected, except for a minor glitch in positioning of the broken-image icon in the vertical-lr cases.
The following 4 (vendor-prefixed) tests with the following parameters for the vertical abs.pos.child box:
5. 'width' is 'auto', 'left' and 'right' are not 'auto' [1]
and the following variations
direction: [ltr | rtl] and writing-mode: [vertical-rl | vertical-lr]
fail in Firefox 40.0a1 buildID=20150430100104:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vrl-022.xht
embedding
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/embedded-doc-abs-pos-non-replaced-icb-vrl-022.html
(no green appears: used width is 0; expected used width 100px)


http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vlr-023.xht
embedding
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/embedded-doc-abs-pos-non-replaced-icb-vlr-023.html
(no green appears: used width is 0; expected used width 100px)


http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vrl-024.xht
embedding
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/embedded-doc-abs-pos-non-replaced-icb-vrl-024.html
(no green appears: used width is 0; expected used width 100px)


http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vlr-025.xht
embedding
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/embedded-doc-abs-pos-non-replaced-icb-vlr-025.html
(no green appears: used width is 0; expected used width 100px)

Chrome 44.0.2383.0 passes these 4 tests.

[1]:
CSS2.1, §10.3.7 Absolutely positioned, non-replaced elements
http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-width
Other (vendor-prefixed) test failures:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vrl-032.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vlr-033.xht


------


I spent well over an hour on 

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vrl-020.xht

and

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-icb-vlr-021.xht

trying to figure out why there is an horizontal scrollbar being generated (which then generates a vertical scrollbar) and I just could not find why. I think this issue is related to <object> in 'direction: ltr' when its embedded document's 'direction' is 'rtl'. And so I think this issue has nothing to do per se with writing-mode implementation. There is possibly a bug report already on this...


------


Another (vendor-prefixed) test failure:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/abs-overconstrained-writing-mode.html 



The beforementioned test failures happen in Firefox 40.0a1 buildID=20150430100104.
Here's a patch incorporating the 96 testcases from comment 1, with corresponding reference files so we can run them as reftests. As noted in comment 1, we currently fail 74 out of the 96 cases. (I have a local WIP patch that currently reduces this to 10 failures, so still a bit more to do...)
Attachment #8606977 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Gérard, I'm looking at http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-017.xht, which is one of the tests that still fails for me with local patches ... and I don't think I understand it. AFAICS, the Firefox rendering (in current Nightly, and also in my locally-patched build) is correct; this also agrees with IE11, although it disagrees with Chrome.

As far as I can tell, in this testcase the <span>s static position should be immediately after the "34" glyphs, on the same (horizontal) line within the containing <div>, just as if no 'position' or 'writing-mode' were applied to it. I don't know why Chrome moves it below, and the explanatory comments in the file don't appear to match the actual code. Could you please double-check this one? Thanks!
Flags: needinfo?(bugzilla)
(In reply to Jonathan Kew (:jfkthame) from comment #9)
> Gérard, I'm looking at
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-
> replaced-vlr-017.xht, which is one of the tests that still fails for me with
> local patches ... and I don't think I understand it. AFAICS, the Firefox
> rendering (in current Nightly, and also in my locally-patched build) is
> correct; this also agrees with IE11, although it disagrees with Chrome.
> 
> As far as I can tell, in this testcase the <span>s static position should be
> immediately after the "34" glyphs, on the same (horizontal) line within the
> containing <div>, just as if no 'position' or 'writing-mode' were applied to
> it.

Yes, agreed. If 'right' is 'auto' in that test, then the <span>s static position should be immediately after the "34" glyphs.

There is an error in that s71-abs-pos-non-replaced-vlr-017 test. Doh!
<title>CSS Writing Modes Test: absolutely positioned non-replaced element - 'direction: ltr' and 'left' and 'width' are 'auto' and 'right' is not 'auto'</title>

(...)

line 37 right: auto;
but it should be 
line 37 right: 2em;

http://test.csswg.org/source/css-writing-modes-3/abs-pos-non-replaced-vlr-017.xht

has the same error!

I also think the background-image is wrong too. It should be 
support/bg-red-2col-2row-320x320.png
and the background-image is wrong too for abs-pos-non-replaced-vrl-016.xht as I think it should be also support/bg-red-2col-2row-320x320.png

Jonathan, I will fix those 4 tests later this evening. I must go now....

Gérard
Flags: needinfo?(bugzilla)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-016.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-017.xht

have been corrected (background-image modified, reference file modified, right: 2em).

Firefox 41 and IE11 now pass these 2 tests.

The correspondent tests in the test suite have also been modified accordingly:
http://hg.csswg.org/test/rev/a224fef2b3ce

Jonathan, let me know if you find any other tests that seems incorrect.

I will random-review approx. 20 of my own s71-abs-pos-non-replaced-* tests today.

Gérard
Okay. I just finished re-examining 19 tests: those ending with a "5" and ending with an "8" 
(eg.
s71-abs-pos-non-replaced-vlr-005  s71-abs-pos-non-replaced-vrl-008
s71-abs-pos-non-replaced-vlr-015  s71-abs-pos-non-replaced-vrl-018
s71-abs-pos-non-replaced-vlr-025  s71-abs-pos-non-replaced-vrl-028
            ......                            ......
s71-abs-pos-non-replaced-vlr-085  s71-abs-pos-non-replaced-vrl-088
s71-abs-pos-non-replaced-vlr-095
)

and I think they are all correct tests.

In a few tests, the comments should be updated to reflect the rules given in §10.6.4 instead of §10.3.7 but that's it.

--------

In this test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-065.xht

Firefox 41 buildID=20150513191426 Inspector tool reports that the computed 'height' of the <span> is 240px (that does not make sense: it should be based on content which is 80px) and computed 'bottom' of the <span> is 0 (that does not make sense: it should be 160px).

--------

Again, Jonathan, if you believe one of my tests is incorrect, let me know. This is appreciated.

Gérard
Comment on attachment 8606977 [details] [diff] [review]
Tests for position:absolute in vertical writing modes, from Gérard Talbot; currently failing 74 out of 96 testcases

Review of attachment 8606977 [details] [diff] [review]:
-----------------------------------------------------------------

I can rs=me if you like, but why don't you just check these in as author=Gérard r=jfkthame?
(In reply to Simon Montagu :smontagu from comment #16)
> Comment on attachment 8606977 [details] [diff] [review]
> Tests for position:absolute in vertical writing modes, from Gérard Talbot;
> currently failing 74 out of 96 testcases
> 
> Review of attachment 8606977 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I can rs=me if you like, but why don't you just check these in as
> author=Gérard r=jfkthame?

OK, that makes sense. FWIW, note that I generated an alternative set of reference files (to avoid comparing an Ahem glyph with a PNG swatch), though unfortunately this still doesn't eliminate all antialiasing fuzz at the glyph edges, especially on Windows. But the actual test files are directly from Gérard's site.

I'll update per comment 14 before checking in, anyway.
(In reply to Gérard Talbot from comment #15)
> In this test
> 
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-
> replaced-vlr-065.xht
> 
> Firefox 41 buildID=20150513191426 Inspector tool reports that the computed
> 'height' of the <span> is 240px (that does not make sense: it should be
> based on content which is 80px) and computed 'bottom' of the <span> is 0
> (that does not make sense: it should be 160px).

Right, that's a clear Gecko bug. It's fixed in the patches I'm working on, but not quite ready for review & landing yet.
Updated the reftest patch with the corrected test files, and made the fuzz annotations specific to OS X and Win/D2D only.
Attachment #8606977 - Attachment is obsolete: true
Attachment #8606977 - Flags: review?(smontagu)
Comment on attachment 8607627 [details] [diff] [review]
Tests for position:absolute in vertical writing modes; currently failing 72 out of 96 testcases

Tests written by Gérard Talbot; new reference files and review=jfkthame.
Attachment #8607627 - Flags: review+
Marking leave-open, as the landing here is just testcases so far; actual patches still to come...
Keywords: leave-open
First part of position:absolute is further changes in nsHTMLReflowState; this builds on top of the patches in bug 1147834. I split this from the nsAbsoluteContainingBlock changes (coming next) to make the patches a more reasonable size, but they should be folded together for landing as they're not really independent.
Attachment #8608578 - Flags: review?(smontagu)
Part 2 of the changes. With this, we pass all 96 of the testcases from comment 1, as well as fixing the remaining reftest regressions from bug 1147834's patches. See try run in comment 26, where the oranges are all the unexpected-passes on the abspos testcases.
Attachment #8608580 - Flags: review?(smontagu)
Finally, we can remove all those failure annotations.
Attachment #8608581 - Flags: review?(smontagu)
Argh, tryserver says there are still some failures on Android; in the comment 26 try run, there were fewer unexpected-passes than on desktop platforms. :(
Depends on: 1167155
No longer depends on: 1167155
(In reply to Jonathan Kew (:jfkthame) from comment #9)
> (...) the explanatory comments in
> the file don't appear to match the actual code.

(In reply to Gérard Talbot from comment #15)
> the comments should be updated to reflect the rules given in
> §10.6.4 instead of §10.3.7 

I am examining, revisiting, trimming and correcting the comments in all 96 tests right now. Almost all of them had incorrect comments...

----------

This test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-093.xht

is incorrect.

"If the values are over-constrained, ignore the value for 'bottom' and solve for that value."
§10.6.4 10.6.4 Absolutely positioned, non-replaced elements
http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-height

Bottom should be ignored and then solved; but the test says top should be ignored and then solved. 
So, in s71-abs-pos-non-replaced-vlr-093.xht

background: red url("support/bg-red-2col-2row-320x320.png")

should be instead

background: red url("support/bg-red-2col-3row-320x320.png")

I must check the other tests...
Okay... I've decided to modify the following tests 

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-062.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-063.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-064.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-065.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-066.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-067.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-068.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-069.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-070.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-071.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-072.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-073.xht

so that they better (more reliably) test how and when to solve height (or width) when top and bottom (or left and right) offsets are constrained, have a defined value.

Chrome 43.0.2357.65 passes all these tests except those (64, 65, 70 and 71)  where the abs. pos. <span> has its writing-mode set to vertical-*.
IE11 passes all the 12 tests mentioned in #c39 except those (72 and 73).
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-092.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-093.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vrl-096.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-097.xht

IE11 fails 096 and 097.

Chrome 44.0.2403.9 and Firefox 41.0a1 buildID=20150513191426 fail all these 4 tests. 

But I'm convinced those 4 uploaded tests are correct.

------------

(In reply to Jonathan Kew (:jfkthame) from comment #29)
> we pass all 96 of the testcases from
> comment 1

I think you need to check your patches against those 4 tests and the 12 tests in #c39 .
I had a talk with fantasai this evening and it appears that I did not understand the following sentence:

"
Layout rules that refer to the *-left and *-right box properties (border, margin, padding, positioning offsets) use *-top and *-bottom instead, and vice versa, mapping the horizontal writing-mode rules of CSS2.1 into vertical writing-mode rules using the flow-relative directions. 
"

therefore I will have to reexamine and reassess (and most likely modify some of) those 96 tests again, probably this week. I am extremely sorry about this unexpected turn of events. I really want these tests (including their comments) to be correct, to be reliable, to be trustworthy.
Attachment #8608578 - Attachment is obsolete: true
Attachment #8608578 - Flags: review?(smontagu)
Attachment #8608580 - Attachment is obsolete: true
Attachment #8608580 - Flags: review?(smontagu)
Updated on top of the current patches in bug 1167930 and bug 1147834. Tryserver (see comment 45) says that this fixes all unit test regressions (reftest failures, assertions) remaining from bug 1147834 by itself.
Here's an updated version of attachment 8599240 [details] that I've been using to experiment interactively with various combinations modes and properties. With the latest patches here, this seems to work well.
Attachment #8599240 - Attachment is obsolete: true
Depends on: 1167930
Attachment #8608581 - Flags: review?(smontagu) → review+
Comment on attachment 8610238 [details] [diff] [review]
patch 1 - Update constraint calculations in nsHTMLReflowState to work with logical coordinates

Review of attachment 8610238 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsHTMLReflowState.cpp
@@ +1024,5 @@
>  
>  // When determining the hypothetical box that would have been if the element
> +// had been in the flow we may not be able to exactly determine both the IStart
> +// and IEnd edges. For example, if the element is a non-replaced inline-level
> +// element we would have to reflow it in order to determine it desired ISize.

s/it/its/ (the second one on the line) while you're here

@@ -1337,2 @@
>  #ifdef DEBUG
> -      aHypotheticalBox.mRightIsExact = true;

AFAICT you still need to set mIEndIsExact to true where appropriate

@@ +1498,3 @@
>    }
> +  if (eStyleUnit_Auto == mStylePosition->mOffset.GetBEndUnit(cbwm)) {
> +    offsets.BEnd(cbwm) = 0;        

Nit: trailing whitespace not removed
Attachment #8610238 - Flags: review?(smontagu) → review+
Attachment #8610239 - Flags: review?(smontagu) → review+
One more issue here: tryserver shows that the vertical-rl testcases fail intermittently (but frequently) on all Android versions. I can reproduce this locally, too. The failures vary from one run to another, even on the exact same build -- although the nature of the failure is always the same, with the abspos element appearing substantially to the right of its correct position -- so I think we need to mark all the vrl testcases as "random" for now.

I'll file a followup bug on this.
Depends on: 1169311
Comment on attachment 8612349 [details] [diff] [review]
patch 4 - Mark vertical-rl testcases as random-if(Android) due to intermittent failures

Cancelling this; the erratic behavior on Android reftests is a result of a bug in patch 1 here, and will be fixed by a corrected version.
Attachment #8612349 - Attachment is obsolete: true
Attachment #8612349 - Flags: review?(smontagu)
Updated part 1 to fix the Android reftest failures etc, so we won't need the followup bug 1169311; better to get it right in the initial landing. :) Carrying forward r=smontagu.
Attachment #8610238 - Attachment is obsolete: true
Gérard, I've been looking at the tests mentioned in comment 41, and am not convinced they are correct. Taking s71-abs-pos-non-replaced-vlr-093.xht, we have

  div#containing-block > span
    {
      background-color: red;
      bottom: 2em;
      color: green;
      height: 1em;
      margin-bottom: 0em;
      margin-top: 0em;
      position: absolute;
      top: 2em;
    }

and the containing block has a height of 4em. So the span is overconstrained, and either the |top| or |bottom| value will have to be discarded and computed from |height| and the other offset.

Your testcase expects |top| to be kept, and |bottom| solved (resulting in 1em), but because in this testcase we have direction:rtl (inherited from the containing block), I think it should be the other way around: the specified |bottom| should be used, and |top| computed to 1em.

The same applies to tests 92, 96 and 97, AFAICS. Please double-check, and let me know if I'm misunderstanding here. Thanks again!
Flags: needinfo?(bugzilla)
(In reply to Jonathan Kew (:jfkthame) from comment #58)
> Gérard, I've been looking at the tests mentioned in comment 41, and am not
> convinced they are correct. 

And your belief is right, is correct.

> Taking s71-abs-pos-non-replaced-vlr-093.xht, we
> have
> 
>   div#containing-block > span
>     {
>       background-color: red;
>       bottom: 2em;
>       color: green;
>       height: 1em;
>       margin-bottom: 0em;
>       margin-top: 0em;
>       position: absolute;
>       top: 2em;
>     }
> 
> and the containing block has a height of 4em. So the span is
> overconstrained, and either the |top| or |bottom| value will have to be
> discarded and computed from |height| and the other offset.
> 
> Your testcase expects |top| to be kept, and |bottom| solved (resulting in
> 1em), but because in this testcase we have direction:rtl (inherited from the
> containing block), I think it should be the other way around: the specified
> |bottom| should be used, and |top| computed to 1em.

The specified 'bottom' should be kept and then 'top' should be ignored and then solved, because 'direction' is 'rtl'. In comment #44, I explained my confusion, misunderstanding.

I agree with your explanation.

> The same applies to tests 92, 96 and 97, AFAICS. Please double-check, and
> let me know if I'm misunderstanding here. Thanks again!

The thing is: the initial version of those tests were probably correct.

I've been sick all week long and had some other urgent task to do this week. Re-visiting and reviewing all 96 abs-pos-non-replaced-v* tests is on my top priority to-do-list right now; I want the /* comments */ to be clear, reliable and trustworthy too.

Gérard
Flags: needinfo?(bugzilla)
Comment on attachment 8612827 [details] [diff] [review]
patch 1 - Update constraint calculations in nsHTMLReflowState to work with logical coordinates

Review of attachment 8612827 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsHTMLReflowState.cpp
@@ +1602,5 @@
>      //  * we're dealing with a replaced element
>      //  * width was constrained by min-width or max-width.
>  
> +    nscoord availMarginSpace =
> +      aCBSize.Width(cbwm) - offsets.IStartEnd(cbwm) - margin.IStartEnd(cbwm) -

Oops...

  s/Width/ISize/

here. Corrected patch coming shortly.
This updates the test files to the current versions from Gérard's site. We still fail most of them, until the patches here (and dependencies) actually land.
Attachment #8612827 - Attachment is obsolete: true
Updated patch 3 to match the corrected testcases; carrying over r=smontagu.
Attachment #8608581 - Attachment is obsolete: true
With the patches above, and Gérard's corrected testcases, we are left with 4 remaining failures:

s71-abs-pos-non-replaced-vlr-065.xht
s71-abs-pos-non-replaced-vlr-071.xht
s71-abs-pos-non-replaced-vrl-064.xht
s71-abs-pos-non-replaced-vrl-070.xht

These have specified 'left' and 'right' offsets, and width:auto; but the test block is orthogonal to its absolute containing block, and so its block-size (which is unconstrained when we call ComputedSize on it) becomes inline-size when we're working in the containing block's writing mode, and we don't handle unconstrained inline-size well. The net result is that the test block, which should end up 1em square, collapses to zero in its block direction (the container's inline direction) due to an NS_UNCONSTRAINEDSIZE in the computations in nsHTMLReflowState::InitAbsoluteConstraints.
Here are a set of testcases to illustrate the issue above. Current trunk code renders only 4 of these cases correctly, and 8 fail. With patches 1 and 2 above, we pass 6, but still fail 6 of the cases.
And here's the patch that makes all 12 examples in attachment 8613299 [details] render correctly, as well as fixing the 4 remaining failures in Gérard's test collection.
Attachment #8613300 - Flags: review?(smontagu)
Now with adjusted fuzz to keep Windows tests happy.
Attachment #8613298 - Attachment is obsolete: true
Attachment #8613300 - Attachment is obsolete: true
Attachment #8613300 - Flags: review?(smontagu)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-replaced-vlr-023.xht

says

line 7: <title> ... and 'right' is not 'auto'</title>
...
line 37: right: auto;

The error is also in

http://test.csswg.org/source/css-writing-modes-3/abs-pos-non-replaced-vlr-023.xht

The background-image and the comments appear correct regardless if 'right' is '2em' or if 'right' is 'auto' in that test.

I will be fixing this test this evening.
Comment on attachment 8613317 [details] [diff] [review]
patch 4 - Handle unconstrained inline-size when computing constraints for an orthogonal absolutely-positioned block

Review of attachment 8613317 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsHTMLReflowState.cpp
@@ +1614,5 @@
> +
> +        // XXX Do these need box-sizing adjustments?
> +        LogicalSize maxSize =
> +          LogicalSize(wm, ComputedMaxISize(),
> +                      ComputedMaxBSize()).ConvertTo(cbwm, wm);

LogicalSize maxSize = ComputedMaxSize(cbwm);
LogicalSize minSize = ComputedMinSize(cbwm);
Attachment #8613317 - Flags: review?(smontagu) → review+
Comment on attachment 8613317 [details] [diff] [review]
patch 4 - Handle unconstrained inline-size when computing constraints for an orthogonal absolutely-positioned block

Review of attachment 8613317 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsHTMLReflowState.cpp
@@ +1614,5 @@
> +
> +        // XXX Do these need box-sizing adjustments?
> +        LogicalSize maxSize =
> +          LogicalSize(wm, ComputedMaxISize(),
> +                      ComputedMaxBSize()).ConvertTo(cbwm, wm);

Good point, and I'll do the same to the parallel block-coordinate code.
Attachment #8613317 - Attachment is obsolete: true
^^ pushed updated/corrected testcases from Gérard; keeping the bug open ready for the actual patches to land.
Depends on: 1147834
http://hg.csswg.org/test/rev/9f35de3f1ab3 ( made, submitted late on may 31st )
corresponds to the revised, corrected-comment tests in the Writing Modes test suite.


I most likely will need to create another batch of 96 tests checking, verifying that layout calculation rules (§10.6.4) that apply to the vertical dimension (in block axis) in horizontal writing modes instead apply to the horizontal dimension (block axis) in vertical writing modes. Because there are no test right now checking this! The added difficulty to take into consideration (when creating those tests) is that 

- block-start in vertical-RL is on the righthand side; so 'top' in §10.6.4 rules maps to 'right' in vRL
 
- block-start in vertical-LR is on the lefthand side; so 'top' in §10.6.4 rules maps to 'left' in vLR

- block-end in vertical-RL is on the lefthand side; so 'bottom' in §10.6.4 rules maps to 'left' in vRL 

- block-end in vertical-LR is on the righthand side; so 'bottom' in §10.6.4 rules maps to 'right' in vLR

Gérard
Keywords: leave-open
Duplicate of this bug: 1158653
(In reply to Gérard Talbot from comment #79)

> I most likely will need to create another batch of 96 tests checking,
> verifying that layout calculation rules (§10.6.4) that apply to the vertical
> dimension (in block axis) in horizontal writing modes instead apply to the
> horizontal dimension (block axis) in vertical writing modes. Because there
> are no test right now checking this! The added difficulty to take into
> consideration (when creating those tests) is that 
> 
> - block-start in vertical-RL is on the righthand side; so 'top' in §10.6.4
> rules maps to 'right' in vRL
>  
> - block-start in vertical-LR is on the lefthand side; so 'top' in §10.6.4
> rules maps to 'left' in vLR
> 
> - block-end in vertical-RL is on the lefthand side; so 'bottom' in §10.6.4
> rules maps to 'left' in vRL 
> 
> - block-end in vertical-LR is on the righthand side; so 'bottom' in §10.6.4
> rules maps to 'right' in vLR

Just for your information.

Another batch of 128 tests verifying that layout calculation rules (§10.6.4) have been created and have been submitted:

with writing-mode: vertical-lr
- - - - - - - - - - - - - - - 

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vlr-103.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vlr-105.htm

...

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vlr-227.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vlr-229.htm

with writing-mode: vertical-rl
- - - - - - - - - - - - - - - 


http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vrl-102.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vrl-104.htm

...

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vrl-226.htm


http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/abs-pos-non-replaced-vrl-228.htm

and Firefox 42.0a1's test results have been gathered too:

http://test.csswg.org/harness/results/css-writing-modes-3_dev/grouped/browser/Firefox/browser_version/42.0/#s7.1

Gérard
(In reply to Gérard Talbot from comment #4)


> I spent well over an hour on 
> 
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-
> replaced-icb-vrl-020.xht
> 
> and
> 
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-abs-pos-non-
> replaced-icb-vlr-021.xht
> 
> trying to figure out why there is an horizontal scrollbar being generated
> (which then generates a vertical scrollbar) and I just could not find why. I
> think this issue is related to <object> in 'direction: ltr' when its
> embedded document's 'direction' is 'rtl'. And so I think this issue has
> nothing to do per se with writing-mode implementation. There is possibly a
> bug report already on this...

Bug 1167911 has been created specifically for those 2 tests.
You need to log in before you can comment on or make changes to this bug.