New issue
Advanced search Search tips

Issue 850216 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 849751



Sign in to add a comment

Scrollbars: fast/text/selection/pre-wrap-overflow-selection.html

Project Member Reported by atotic@chromium.org, Jun 6 2018

Issue description

fast/text/selection/pre-wrap-overflow-selection.html has an extra scrollbar.

The extra scrollbar appears because 1st linebox is too wide:

Box (block-flow children-inline) offset:unplaced size:108x83
 LineBox offset:4,4 size:124x15  InkOverflow: 0,0 125.828x15
  Text offset:0,0 size:85x15  InkOverflow: 0,0 86x15  visualRect: 0,0 86x15 start: 0 end: 11
  Text offset:85,0 size:39x15  InkOverflow: 0,0 40x15  visualRect: 0,0 40x15 start: 11 end: 16

This is the <pre> text:
----
<pre id="t2" wrap style="overflow:auto; border:4px solid brown; width:100px">
This   text     will wrap
and   fit   within the
box.
</pre>

---

The first line gets two Text fragments: 

"This   text", width 85
"     ", width 40

which causes the overflow.

Legacy seems to be doing something funny where 2nd text fragment holds 5 spaces, but only 2 are visible.
 
Cc: kojii@chromium.org
Status: Available (was: Untriaged)
Blocking: 849751

Comment 3 by kojii@chromium.org, Jun 12 2018

Cc: -kojii@chromium.org
Owner: kojii@chromium.org
Status: Assigned (was: Available)
Thanks, looks like inline layout problem.

In pre-wrap, trailing whitespace should not cause a layout overflow, but if non-space characters overflow, it should be a layout overflow. Maybe I should cut down the line box size to not to include trailing spaces? I'll run some more tests to see how to fix this.

Comment 4 by kojii@chromium.org, Jun 12 2018

Tested more.

* Blink shows scrollbars if contenteditable, otherwise doesn't.
* Gecko doesn't shows scrollbars regardless contenteditable.
* Edge/IE shows scrollbars regardless contenteditable.

WebKit implemented this behavior in https://bugs.webkit.org/show_bug.cgi?id=5619, probably for web-compat in 2005, but we can probably align to Edge today? I'll send CL to ask opinions.

Comment 5 by atotic@chromium.org, Jun 12 2018

My vote is for Edge behavior. It is the least quirky behavior. Other interesting things with hidden spaces is that they can be selected, but selection is invisible.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da403bb0f9b980debd35d2b23312c7d6822bb941

commit da403bb0f9b980debd35d2b23312c7d6822bb941
Author: Koji Ishii <kojii@chromium.org>
Date: Wed Jun 13 15:52:55 2018

[LayoutNG] Let scrollbars to appear for trailing spaces

fast/text/selection/pre-wrap-overflow-selection.html
is a test for scrollbars to not to appear for trailing spaces
when 'white-space: pre-wrap' is used.

* Blink does not show scrollbars for trailing spaces, but
  does show if contenteditable.
* Gecko does not show scrollbars regardless of
  contenteditable.
* Edge/IE show scrollbars regardless of contenteditable.
* WebKit is the same as Blink, allows scroll only if
  contenteditable.

This behavior was implemented in 2006[1], but given Edge/IE
show scrollbars, and different behavior for contenteditable
isn't preferred, this patch rebaselines to show scrollbars
regardless of contenteditable, matching to Edge/IE.

[1] https://bugs.webkit.org/show_bug.cgi?id=5619

Bug:  850216 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I0c8623c90783e44b21b9e5ad99dda6c69b9279a7
Reviewed-on: https://chromium-review.googlesource.com/1097238
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566847}
[modify] https://crrev.com/da403bb0f9b980debd35d2b23312c7d6822bb941/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[add] https://crrev.com/da403bb0f9b980debd35d2b23312c7d6822bb941/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/text/selection/pre-wrap-overflow-selection-expected.png

Comment 7 by kojii@chromium.org, Jun 13 2018

Status: Fixed (was: Assigned)
Let's call it "fixed", at least until web-compat proves that this needs the Blink behavior.

Sign in to add a comment