Issue 2893833004: When moving past a left-hand scrollbar, don't jump way outside the content box.

mstensho (USE GERRIT)

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

3 years, 7 months ago (2017-05-19 09:40:23 UTC) #1

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2893833004/1

3 years, 7 months ago (2017-05-19 09:40:53 UTC) #2

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

3 years, 7 months ago (2017-05-19 11:32:51 UTC) #3

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/449717)

3 years, 7 months ago (2017-05-19 11:32:51 UTC) #4

mstensho (USE GERRIT)

mstensho@opera.com changed reviewers: + cbiesinger@chromium.org, eae@chromium.org

3 years, 7 months ago (2017-05-19 11:43:34 UTC) #5

mstensho (USE GERRIT)

Scrollbars really act up when put on a block that's smaller than the scrollbar itself. ...

3 years, 7 months ago (2017-05-19 11:43:35 UTC) #6

cbiesinger

I guess this is not terrible but wouldn't we be better off changing UpdateLogicalWidth to ...

3 years, 7 months ago (2017-05-19 16:20:31 UTC) #7

mstensho (USE GERRIT)

On 2017/05/19 16:20:31, cbiesinger wrote: > I guess this is not terrible but wouldn't we ...

3 years, 7 months ago (2017-05-19 17:30:36 UTC) #8

cbiesinger

On 2017/05/19 17:30:36, mstensho wrote: > On 2017/05/19 16:20:31, cbiesinger wrote: > > I guess ...

3 years, 7 months ago (2017-05-19 21:50:45 UTC) #9

mstensho (USE GERRIT)

On 2017/05/19 21:50:45, cbiesinger wrote: > On 2017/05/19 17:30:36, mstensho wrote: > > On 2017/05/19 ...

3 years, 7 months ago (2017-05-22 09:29:30 UTC) #10

On 2017/05/19 21:50:45, cbiesinger wrote:
> On 2017/05/19 17:30:36, mstensho wrote:
> > On 2017/05/19 16:20:31, cbiesinger wrote:
> > > I guess this is not terrible but wouldn't we be better off changing
> > > UpdateLogicalWidth to ensure that our width is never <
> > Border+Padding+Scrollbar?
> > 
> > I'm pretty sure logical width has to be strict border-box width. So
> > "overflow:scroll; width:1px; box-sizing:border-box;" should really give 1px,
> not
> > <width of scrollbar>px.
> 
> Why is that so? It is not what you get with "width: 1px; padding-left: 20px;
> box-sizing: border-box;":
> 
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAz1PHQIlMCQ1OjEmAZBOTR+VHXikTwGqAQ9H...

Right, "strict border-box width" was a really sloppy term, and mostly wrong. :)

https://www.w3.org/TR/css-ui-3/#box-sizing

Since the content box width cannot be negative, actual border-box size may be
larger than specified.

Regarding scrollbars:

https://drafts.csswg.org/css2/visufx.html#propdef-overflow

So the scrollbar should be wedged between the inner border and outer padding
edge on some side, and be subtracted from the containing block size established
by the object. Negative sizes should not be allowed anywhere here, though. Apart
from that, there seems to be two ways to interpret the spec (or maybe one camp
is simply violating it). Chrome and Firefox currently don't let the natural
width of the scrollbar affect the size of the border box. That's either a spec
violation, or we could get away with it by saying that in extreme
size-constrained situations, we just shrink the size of the scrollbar to fit
within the space we have. This is certainly not what's happening visually in
Chrome, but maybe no big deal. IE and Edge on the other hand, reserve space for
the scrollbars inside its border box. Which indeed seems like a more
straight-forward interpretation of the spec.

<!DOCTYPE html>
<p>Should there be any hotpink below?</p>
<div style="float:left; background:hotpink;">
    <div style="width:1px; height:40px; background:yellow;"></div>
    <div style="overflow:scroll; width:1px; height:40px;
background:blue;"></div>
</div>

So one option is indeed to start behaving more like IE / Edge. That may have
larger (compatibility) implications than my current CL, though, since this CL
here is just about reversing some ill effects of a bugfix. I suspect such a fix
(stretch border-box to encompass the full width of any scrollbars) would require
some more bake-time, though. I intended this CL for Chrome 60, to fix problems
with AngularJS.

mstensho (USE GERRIT)

On 2017/05/22 09:29:30, mstensho wrote: > On 2017/05/19 21:50:45, cbiesinger wrote: > > On 2017/05/19 ...

3 years, 7 months ago (2017-05-23 14:19:13 UTC) #11

On 2017/05/22 09:29:30, mstensho wrote:
> On 2017/05/19 21:50:45, cbiesinger wrote:
> > On 2017/05/19 17:30:36, mstensho wrote:
> > > On 2017/05/19 16:20:31, cbiesinger wrote:
> > > > I guess this is not terrible but wouldn't we be better off changing
> > > > UpdateLogicalWidth to ensure that our width is never <
> > > Border+Padding+Scrollbar?
> > > 
> > > I'm pretty sure logical width has to be strict border-box width. So
> > > "overflow:scroll; width:1px; box-sizing:border-box;" should really give
1px,
> > not
> > > <width of scrollbar>px.
> > 
> > Why is that so? It is not what you get with "width: 1px; padding-left: 20px;
> > box-sizing: border-box;":
> > 
> >
>
http://plexode.com/eval3/#s=aekVQXANJVQMbAz1PHQIlMCQ1OjEmAZBOTR+VHXikTwGqAQ9H...
> 
> Right, "strict border-box width" was a really sloppy term, and mostly wrong.
:)
> 
> https://www.w3.org/TR/css-ui-3/#box-sizing
> 
> Since the content box width cannot be negative, actual border-box size may be
> larger than specified.
> 
> Regarding scrollbars:
> 
> https://drafts.csswg.org/css2/visufx.html#propdef-overflow
> 
> So the scrollbar should be wedged between the inner border and outer padding
> edge on some side, and be subtracted from the containing block size
established
> by the object. Negative sizes should not be allowed anywhere here, though.
Apart
> from that, there seems to be two ways to interpret the spec (or maybe one camp
> is simply violating it). Chrome and Firefox currently don't let the natural
> width of the scrollbar affect the size of the border box. That's either a spec
> violation, or we could get away with it by saying that in extreme
> size-constrained situations, we just shrink the size of the scrollbar to fit
> within the space we have. This is certainly not what's happening visually in
> Chrome, but maybe no big deal. IE and Edge on the other hand, reserve space
for
> the scrollbars inside its border box. Which indeed seems like a more
> straight-forward interpretation of the spec.
> 
> <!DOCTYPE html>
> <p>Should there be any hotpink below?</p>
> <div style="float:left; background:hotpink;">
>     <div style="width:1px; height:40px; background:yellow;"></div>
>     <div style="overflow:scroll; width:1px; height:40px;
> background:blue;"></div>
> </div>
> 
> So one option is indeed to start behaving more like IE / Edge. That may have
> larger (compatibility) implications than my current CL, though, since this CL
> here is just about reversing some ill effects of a bugfix. I suspect such a
fix
> (stretch border-box to encompass the full width of any scrollbars) would
require
> some more bake-time, though. I intended this CL for Chrome 60, to fix problems
> with AngularJS.

Tried it here: https://codereview.chromium.org/2897033002/

Not sure if I updated all the places in the code. One of the regressions -
fast/text/find-hidden-text.html - is a bit concerning, while the rest probably
just need a rebaseline.

cbiesinger

so lgtm to fix the Angular issue but I do think that something like https://codereview.chromium.org/2897033002/ ...

3 years, 7 months ago (2017-05-23 21:15:32 UTC) #12

mstensho (USE GERRIT)

The CQ bit was checked by mstensho@opera.com

3 years, 7 months ago (2017-05-24 05:50:35 UTC) #13

commit-bot: I haz the power

CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2893833004/1

3 years, 7 months ago (2017-05-24 05:51:24 UTC) #14

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

3 years, 7 months ago (2017-05-24 08:52:45 UTC) #15

commit-bot: I haz the power

Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/301612)

3 years, 7 months ago (2017-05-24 08:52:46 UTC) #16

mstensho (USE GERRIT)

The CQ bit was checked by mstensho@opera.com

3 years, 7 months ago (2017-05-24 09:00:50 UTC) #17

commit-bot: I haz the power

CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2893833004/1

3 years, 7 months ago (2017-05-24 09:01:23 UTC) #18

commit-bot: I haz the power

CQ is committing da patch. Bot data: {"patchset_id": 1, "attempt_start_ts": 1495616450785420, "parent_rev": "b69b3599eedc856b722158672add9095a518440c", "commit_rev": "d457f863f78941ed376b463ee39476b1a1852797"}

3 years, 7 months ago (2017-05-24 09:45:07 UTC) #19

commit-bot: I haz the power

Description was changed from ========== When moving past a left-hand scrollbar, don't jump way outside ...

3 years, 7 months ago (2017-05-24 09:45:26 UTC) #20

commit-bot: I haz the power

3 years, 7 months ago (2017-05-24 09:45:27 UTC) #21

Message was sent while issue was closed.

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/d457f863f78941ed376b463ee394...

Issue 2893833004: When moving past a left-hand scrollbar, don't jump way outside the content box. (Closed)

Description

Patch Set 1 #

Messages