Bug 24434 – WebKit/win fails LayoutTests/fast/forms/textarea-scrollbar-height.html

• Status: REOPENED
• Component: Text

[Peter Kasting]

LayoutTests/fast/forms/textarea-scrollbar-height.html does not pass on WebKit for Windows.  It passed in Safari 3, presumably due to Mac-style font metrics/rendering.

This test also fails in Chromium.

Real-world page that's affected: http://de.news.yahoo.com/wirtschaft.html , see text in right navigation strip saying "Wirtschaft: Alle Videos »" that has an unnecessary scrollbar.

[Victor Wang]

Created attachment 40255 [details]
Screenshots

[Victor Wang]

In this case, the CSS line height is 15 (13*1.22) and the text font height is 16 on windows (13 + 3 internal leading for accent marks etc), so it is actually overflowed by 1 pixel. This is why Webkit for windows displays a scrollbar. The same thing could happen to Webkit on MAC if the line height number <= 1.15 (The text height on mac for this case is 15). Repro on safari if line height changes to <= 1.15.

Here are two places where Webkit is different from IE and Firefox:

1. The CSS line height calculation:
IE and firefox round the line height number while webkit truncates the fractional digits, so in this case (13*1.22 = 15.86), IE and Firefox return 16 and webkit returns 15.

2. Scroll bar display (See attached image)
-. IE does not display vertical scroll bar if the text item overflows;
-. Firfox does not display scroll bar for most cases unless the total number of lines > certain threshold. The scroll bar does not seem help much when it is displayed (see the last screenshot in attached image).
-. Webkit displays a scroll bar with > 0 pixel overflow.

In this case, the scroll bar does not seem very helpful and the IE behavior looks better to me, should we remove the scroll bar for list item?

Any comments?

[Dave Hyatt]

Rounding non-integer line height values sounds reasonable to me.

[Victor Wang]

Created attachment 40480 [details]
Proposed patch - round line height

[Dave Hyatt]

Comment on attachment 40480 [details]
Proposed patch - round line height

r=me

[Victor Wang]

Patch that rounds non-integer line height was landed. It fixes this layout test.
another issue is whether or not to display vertical bar when list item overflow a few pixels. Another bug was filed for it: https://bugs.webkit.org/show_bug.cgi?id=30361

[David Kilzer (:ddkilzer)]

Fixed in r49567.  <http://trac.webkit.org/changeset/49567>

[Darin Adler]

This change broke the layout of the Dashboard Widget named “Business” included with Mac OS X.

[Darin Adler]

(In reply to comment #8)
> This change broke the layout of the Dashboard Widget named “Business” included
> with Mac OS X.

It has a line-height of 1.5 that applies to some blocks with a font size of 9px, and the layout of the widget was relying on the resulting line height being 13px rather than of 14px.

(In reply to comment #9)
> (In reply to comment #8)
> > This change broke the layout of the Dashboard Widget named “Business” included
> > with Mac OS X.
> 
> It has a line-height of 1.5 that applies to some blocks with a font size of
> 9px, and the layout of the widget was relying on the resulting line height
> being 13px rather than of 14px.

I just tried this test case
    <div style="line-height: 1.5; font-size: 9px; background-color:
yellow">a</div>
in Firefox (3.5.6) and got a 13px tall block.

In Firefox, line-height: 1.54351 still gives 13px, but 1.54352 gives 14px.

(In reply to comment #11)
> In Firefox, line-height: 1.54351 still gives 13px, but 1.54352 gives 14px.

Actually, Firefox stores and reports the height (in the case of line-height: 1.5) as 13.5px, and the pixel height on screen depends on where the block is positioned (presumably the render tree uses fractional coordinates and they are rounded to integers closer to render time).

Reverted r49567 in <http://trac.webkit.org/projects/webkit/changeset/53450> due it breaking the Business widget.

[Victor Wang]

Hi Dan and Darin,

Thanks for taking time looking into this and sorry for breaking the Businees widget (was off yesterday so did not reply earlier).

The test failed is textarea-scrollbar-height.html, and I was checking the clientHeight on different platform to see why this test fails. A couple of examples show me that IE and Firefox are rounding this number, so I thought the line-height is also rounded. Per Dan, looks like FireFox stores floating number as line-height and rounds the result when calculating the clientHeight. In webkit, line-height is truncated and returned as integer, this causes the clientHeight to be truncated and the test fails on Windows. Do you have any suggestions on how to fix the test?

Thanks,
Victor

[Peter Kasting]

Note that per comment 0 this behavior affects real-world sites too, so it's not just a matter of changing a test.

I think we should match IE and Firefox if possible.  I would be unhappy to end up in a place where all WebKit-based user agents had to prioritize a particular OS X dashboard widget over the rest of the web, although it's not clear to me that this has come to that.  (If it did come to that, I'd suggest putting this behavior under a flag, turning that flag on only for dashboard widgets, expiring it at a particular WebKit revision number, and shipping a new version of the dashboard widget that checks the revision number.)

[Darin Adler]

Sure, we would like to match the other browsers.

Dan’s testing showed the patch did not make us match Firefox, although it made us closer.

We may have to create quirks that keep content that is depending on Safari’s old behavior working. The Dashboard widget is one example; there may be others. Some approach to this does have to be part of the patch now that we’re aware of the issue.

[Darin Adler]

The original comment describes this as a Safari 3 to Safari 4 regression. Could someone comment more on why that page worked in Safari 3?

[Peter Kasting]

Clarity: The testcase passed in Safari 3/Win.  I don't know if the real-world page looked right or not (I don't recall having tested it).  My suspicion at the time was that the testcase passed because Safari 3/Win defaulted to Mac-style font rendering/metrics (which didn't trip this bug, see comment 2) and Safari 4/Win defaulted to Windows-style font rendering/metrics.

[Eric Seidel (no email)]

Comment on attachment 40480 [details]
Proposed patch - round line height

Marking this patch as r- since it was reverted.