Closed Bug 1141867 Opened 7 years ago Closed 7 years ago

Layout of contiguous floats and vertical writing-modes

Categories

(Core :: Layout: Floats, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bugzilla, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(4 files, 1 obsolete file)

Tests (with vendor-prefixes)
----------------------------

A)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-002.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-004.xht

B)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-012.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-013.xht

Expected results
----------------

A)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s43-inline-replaced-vrl-004-ref.xht

and

B)
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-012-ref.xht

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

The main, primary goal of those tests was to verify this sentence-rule for laying out horizontally contiguous floats:
"
 If the current box is left-floating, and there are any left-floating boxes generated by elements earlier in the source document, then for each such earlier box, either the left outer edge of the current box must be to the right of the right outer edge of the earlier box, or its top must be lower than the bottom of the earlier box. Analogous rules hold for right-floating boxes. 
"
CSS2.1, §9.5.1 Positioning the float: the 'float' property
http://www.w3.org/TR/CSS21/visuren.html#float-position

Notes
-----
- http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-012.xht
seems like a duplicate test of
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/wm-line-box-direction-006a.xht
- Those tests (along with 8 other float-contiguous tests) have been submitted to the test.csswg.org repository 
http://lists.w3.org/Archives/Public/public-css-testsuite/2015Mar/0001.html
but have not yet been reviewed
- IE11, Chrome 40.0.2272.76 pass these 4 tests. Prince 9.0.5 seems to pass the 3 vertical-rl tests.
- I am using Firefox 39.0a1 buildID=20150306184824
- I use Linux 3.16.0-30-generic x86_64, Qt: 4.8.6, KDE 4.14.1; Kubuntu (utopic) 14.10
- I've searched for duplicates and did not find any.
Blocks: writing-mode
Keywords: testcase
The "A" tests pass in a build with the patches from bug 1145218.
Depends on: 1145218
This is a slightly simplified version of Gérard's tests, using an explicitly-specified size for the floats.

It seems there are (at least) two issues to be addressed here. The positioning of the floats is wrong because FloatMarginISize() (helper in nsBlockReflowState) needs to return the inline-size in the containing block reflow state's writing mode, which is NOT necessarily the float's mode.

Fixing this (patch coming) will fix the rendering of this testcase; however, by itself it doesn't fix Gérard's original. There, we also have an issue with computing the used (auto) block-size of the floats; I'm still looking into that.
Here's the patch that fixes attachment 8590092 [details] (but not the (B) tests from comment 0).
Attachment #8590095 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
The second problem arises when the (orthogonal) float's block-size is unconstrained; this means FloatMarginISize can't return a useful result based on ComputeSize(). Instead, we'll need to actually reflow the float block to find its desired size. With this patch, all the testcases in this bug now pass successfully.
Attachment #8590129 - Flags: review?(smontagu)
And here are some reftests. I'm expecting a certain amount of fuzz will be needed, as these tests compare Ahem glyphs to solid background blocks, which means we'll get antialiasing discrepancies at the edges; once tryserver reopens, I'll see how it looks.
Attachment #8590148 - Flags: review?(smontagu)
Now with fuzz annotations as suggested by tryserver.
Attachment #8590257 - Flags: review?(smontagu)
Attachment #8590148 - Attachment is obsolete: true
Attachment #8590148 - Flags: review?(smontagu)
Attachment #8590095 - Flags: review?(smontagu) → review+
Attachment #8590129 - Flags: review?(smontagu) → review+
Attachment #8590257 - Flags: review?(smontagu) → review+
Cancelling n-i on gtalbot; the problem here is with the adapted version of his testcases that I landed. I'll fix.
Flags: needinfo?(bugzilla)
In tests

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vrl-012.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-float-contiguous-vlr-013.xht

, I use Ahem font and have only 5 characters per each floating <div>s.

In layout/reftests/floats/orthogonal-floats-1a.html and orthogonal-floats-1b tests, Ahem font is not used and

    1.45 -  <div class="floated-right">fghijk</div>
    1.46 -
    1.47 -  <div class="floated-right">lmnopq</div>

those 2 <div>s have 6 characters.

orthogonal-floats-1c and orthogonal-floats-1d appear correct: they use Ahem font and the <div> have 5 characters.

I hope this helps you.

Gérard
Comment on attachment 8590257 [details] [diff] [review]
patch 3 - Reftests for contiguous orthogonal floats

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

::: layout/reftests/floats/orthogonal-floats-1c.html
@@ +3,5 @@
> + <head>
> +  <title>Bug 1141867 - Contiguous right-floating boxes with vertical writing mode</title>
> +  <!-- based on testcases from Gérard Talbot, see https://bugzilla.mozilla.org/show_bug.cgi?id=1141867 -->
> +  <style type="text/css">
> +  <meta charset="utf-8">

Oops - the problem here was that when I added a <meta charset...> tag to the test files, I accidentally put it *after* the opening <style> tag in this case, which causes the entire @font-face declaration to be dropped. And so test 1c failed everywhere.
Depends on: 1157691
You need to log in before you can comment on or make changes to this bug.