1130907 - float and shrink-to-fit in vertical writing-modes

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect)

Description

[Gérard Talbot]

Tests
-----

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-basic.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-both-borders.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-both-horizontal-border-padding.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-016.xht

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

There is no red

Explanation
-----------
In case a floated non-replaced element (or an inline-block non-replaced element) has 'height: auto' (which is the logical width) in a vertical writing-mode, then shrink-to-fit algorithm must apply to determine its used height (logical width).
CSS2.1, §10.3.5 Floating, non-replaced elements
http://www.w3.org/TR/2011/REC-CSS2-20110607/visudet.html#float-width
and CSS3 Writing-Modes, §7.1 Principles of Layout in Vertical Writing Modes
"
Layout calculation rules (such as those in CSS2.1, Section 10.3) that apply to the horizontal dimension in horizontal writing modes instead apply to the vertical dimension in vertical writing modes.
"
There are 2 other situations (CSS2.1, §10.3.7) where shrink-to-fit algorithm must also apply.
In the 4 tests, the used height (logical width) of each and all floating elements should be 0; therefore, no red should be rendered.


Notes
-----
- I have not yet submitted those tests to the test.csswg.org repository but I will, along with correspondent reference files
- Those tests' filenames will most likely change when I submit them to test.csswg.org repository
- IE11, Chrome 40.0.2214.93 and Prince 9.0.5 pass these 4 tests.
- I am using Firefox 38.0a1 buildID=20150204150323
- 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

Comment 1

[Gérard Talbot]

In this test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-789.xht

(which is passed by Chrome 40.0.2214.111, IE11 and Prince 9.0.5)

the glyphs "L" and "R" appear to be not rendered at all: after dynamically setting the font-family to serif, the "L" and "R" should become viewable... but they will not. 
The logical width of the content is zero. So there is something wrong with the handling of content. If content is 123456, then only 12345 is rendered; the last glyph is not rendered. 

I can file this issue into a separate bug report...

Comment 2

[Gérard Talbot]

Better test:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-790.xht

The content (the "6" glyph and a part of "5" glyph; it depends on the installed serif font) at line-end is overlapping the blue border-bottom in Firefox 38.0. Not so in Chrome 40.0.2214.111, IE11 and Prince 9.0.5.

Comment 3

[Simon Montagu :smontagu]

For a start, we still have some holdouts in nsHTMLReflowState that are using height and width instead of abstract sizes when calculating size constraints.

Comment 4

[Gérard Talbot]

> - I have not yet submitted those tests to the test.csswg.org repository but
> I will, along with correspondent reference files

Done.

http://lists.w3.org/Archives/Public/public-css-testsuite/2015Mar/0001.html

> - Those tests' filenames will most likely change when I submit them to
> test.csswg.org repository

The tests at/in gtalbot.org/BrowserBugsSection/CSS3WritingModes/ use vendor-prefixed property names.

The tests' new filenames at test.csswg.org use unprefixed property names:

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

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-003.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-005.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-007.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-009.htm

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

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-002.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-004.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-006.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-008.htm

Both vertical writing-modes
 - - - - - - - - - - - - - 

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-vlr-016.htm

Comment 5

[Simon Montagu :smontagu]

Bug 1145218 is probably also a blocker for this.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Account for writing-mode when incorporating border widths into intrinsic size

It turns out the primary issue here is that nsFrame::IntrinsicISizeOffsets, which is used to accumulate border contributions to intrinsic size, hasn't been made vertical-aware. Fixing this makes all the testcases here render as expected, AFAICT.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: testcase for intrinsic sizing that includes borders

Here's a testcase that demonstrates the incorrect intrinsic sizing we get (as shown by the size of the floated divs) with current code, and displays correctly once this is fixed.

(Should be possible to turn this into a reftest...)

Comment 8

[Jonathan Kew [:jfkthame]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=09de9e4d36e0

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Reftest for contribution of border widths to intrinsic size

Here's a reftest to go with the patch above.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: testcase for nsContainerFrame intrinsic size involving borders

It turns out there's a similar problem in nsContainerFrame, although the original testcases here didn't show it. Here's a modified testcase that renders incorrectly even after the nsFrame::IntrinsicISizeOffsets patch.

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: patch 2 - Account for writing-mode in nsContainerFrame::DoInlineIntrinsicISize

And here's the patch that fixes that second example.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: Reftest for container-frame intrinsic size involving borders

Comment 13

[Jonathan Kew [:jfkthame]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=140c47a6dc0c

Comment 14

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #13)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=140c47a6dc0c

Note that none of the oranges in this try run are from this bug; they're the tests from bug 1150614, which was also in my queue at the time.

Comment 15

[Simon Montagu :smontagu]

Comment on attachment 8587931 [details] [diff] [review]
Account for writing-mode when incorporating border widths into intrinsic size

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

Nice

Comment 16

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/289e6aa106f5
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d778faf638
https://hg.mozilla.org/integration/mozilla-inbound/rev/67e118ce909f
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5a10278abfb

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/289e6aa106f5
https://hg.mozilla.org/mozilla-central/rev/c0d778faf638
https://hg.mozilla.org/mozilla-central/rev/67e118ce909f
https://hg.mozilla.org/mozilla-central/rev/d5a10278abfb

Comment 18

[Gérard Talbot]

(In reply to Jonathan Kew (:jfkthame) from comment #10)
> Created attachment 8587980 [details]
> testcase for nsContainerFrame intrinsic size involving borders
> 
> It turns out there's a similar problem in nsContainerFrame, although the
> original testcases here didn't show it. Here's a modified testcase that
> renders incorrectly even after the nsFrame::IntrinsicISizeOffsets patch.

Jonathan,

Are you sure about that test? I ask because that would cause a noticeable difference between how inline boxes with border and padding would be rendered in vertical writing-modes versus how they are rendered in horizontal writing-mode.

CSS 1, Section 4.4 The height of lines says 
"any padding, border or margin above and below non-replaced inline elements does not influence the height of the line. In other words: if the 'line-height' is too small for the chosen padding or border, it will overlap with text on other lines."
http://www.w3.org/TR/CSS1/#the-height-of-lines

CSS 2.1, Section 10.8.1 Leading and half-leading says 
"The height of the inline box encloses all glyphs and their half-leading on each side and is thus exactly 'line-height'. Boxes of child elements do not influence this height.

Although margins, borders, and padding of non-replaced elements do not enter into the line box calculation, they are still rendered around inline boxes. This means that if the height specified by 'line-height' is less than the content height of contained boxes, backgrounds and colors of padding and borders may 'bleed' into adjoining line boxes."
http://www.w3.org/TR/CSS21/visudet.html#line-height

Tests
-----

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-vlr-003-draft.xht

I think IE11 renders this test as I think it should be. 
Firefox 42 does not. 
Chrome 45 is very wrong in my opinion.

Here's a test I created several years ago, testing the quoted and specified statements of the spec:

http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/NestedInlineElements.html

Gérard

Comment 19

[Gérard Talbot]

Exact same test as sizing-inline-content-with-border-vlr-003-draft but without 'float: left' :

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-vlr-003-draft2.xht

and again, in my opinion, IE11 renders it correctly. Firefox 42 buildID=20150718201542 and Chrome 45.0.2454.12 do not.

Comment 20

[Gérard Talbot]

An interactive demo test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-dhtml.xht

only working in Firefox 41+ and in Chrome 43+

Comment 21

[Jonathan Kew [:jfkthame]]

Note that these sizing-inline tests may be at least partly dependent on baselines and how vertical-align is handled. The behavior is noticeably different if you set 'text-orientation: sideways-right" on the examples, which makes baseline alignment behave more like it does in horizontal mode.

This may be related to the issues in bug 1179952; and if not, it'd be best to file a separate bug with the relevant examples - thanks.

Comment 22

[fantasai]

No, it's not related to bug 1179952 -- that's about inline-blocks, and these are just regular inlines. It's clearly an alignment bug, but also clearly a separate bug from this one.

Comment 23

[Gérard Talbot]

(In reply to Jonathan Kew (:jfkthame) from comment #21)
> Note that these sizing-inline tests may be at least partly dependent on
> baselines and how vertical-align is handled.

I do not think so. Whether dominant baseline is central or alphabetical should not affect how border and padding of inline non-replaced elements is rendered (or is supposed to be rendered) in a line box. Which baseline-alignment method is involved is one distinct and separate thing; padding and border of inline non-replaced elements should not affect (increase, stretch) line box height.

> The behavior is noticeably
> different if you set 'text-orientation: sideways-right" on the examples,
> which makes baseline alignment behave more like it does in horizontal mode.
> 
> This may be related to the issues in bug 1179952; and if not, it'd be best
> to file a separate bug with the relevant examples - thanks.

I will file a separate bug with examples on this: it's one item in my very long to-do-list now.

Comment 24

[Gérard Talbot]

> I will file a separate bug with examples on this

Bug 1220352 has been filed: "Line box height should not grow to accomodate border and padding of non-replaced inline boxes"