Bug 213739

Summary:

[css-flexbox] Don't include scrollbar extents when computing sizes for percentage resolution

Product:

WebKit

Reporter:

Sergio Villar Senin <svillar>

Component:

New Bugs

Assignee:

Sergio Villar Senin <svillar>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

213809

Bug Blocks:

Attachments:

Description	Flags
Patch	jfernandez: review+

Description Sergio Villar Senin 2020-06-29 09:44:25 PDT

[css-flexbox] Don't include scrollbar extents when computing sizes for percentage resolution

Comment 1 Sergio Villar Senin 2020-06-29 09:47:27 PDT

Created attachment 403075 [details]
Patch

Comment 2 Javier Fernandez 2020-06-29 14:33:43 PDT

Please, make sure the patch is not the cause of some of the flakiness detected by the mac2 bots: 

fast/scrolling/mac/scroll-snapping-in-progress.html

Comment 3 Manuel Rego Casasnovas 2020-06-30 01:05:02 PDT

Comment on attachment 403075 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403075&action=review

BTW, about the confusion regarding storing something different than what its name says, in Chromium the solution for that was https://chromium-review.googlesource.com/c/chromium/src/+/1024171/, maybe we should try to port that or something similar into WebKit.

> LayoutTests/ChangeLog:8
> +        * TestExpectations: Unskipped percentage-heights-004.html.

The test is only for heights, wouldn't we need a test for widths too? You're changing both things in the code in I'm not getting it wrong.

Comment 4 Sergio Villar Senin 2020-06-30 01:22:26 PDT

(In reply to Javier Fernandez from comment #2)
> Please, make sure the patch is not the cause of some of the flakiness
> detected by the mac2 bots: 
> 
> fast/scrolling/mac/scroll-snapping-in-progress.html

That's failing without the patch as well.

Comment 5 Sergio Villar Senin 2020-06-30 10:23:05 PDT

(In reply to Manuel Rego Casasnovas from comment #3)
> Comment on attachment 403075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403075&action=review
> 
> BTW, about the confusion regarding storing something different than what its
> name says, in Chromium the solution for that was
> https://chromium-review.googlesource.com/c/chromium/src/+/1024171/, maybe we
> should try to port that or something similar into WebKit.
> 
> > LayoutTests/ChangeLog:8
> > +        * TestExpectations: Unskipped percentage-heights-004.html.
> 
> The test is only for heights, wouldn't we need a test for widths too? You're
> changing both things in the code in I'm not getting it wrong.

Now that you mention it, the width paths are not reachable. I'll post a prequel patch to remove them.

Comment 6 Sergio Villar Senin 2020-07-01 03:41:11 PDT

Committed r263794: <https://trac.webkit.org/changeset/263794>

Comment 7 Radar WebKit Bug Importer 2020-07-01 03:42:14 PDT

<rdar://problem/64980642>

Comment 8 Sergio Villar Senin 2020-07-01 03:45:02 PDT

(In reply to Manuel Rego Casasnovas from comment #3)
> Comment on attachment 403075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403075&action=review
> 
> BTW, about the confusion regarding storing something different than what its
> name says, in Chromium the solution for that was
> https://chromium-review.googlesource.com/c/chromium/src/+/1024171/, maybe we
> should try to port that or something similar into WebKit.
> 
> > LayoutTests/ChangeLog:8
> > +        * TestExpectations: Unskipped percentage-heights-004.html.
> 
> The test is only for heights, wouldn't we need a test for widths too? You're
> changing both things in the code in I'm not getting it wrong.

After landing bug 213809 there is no need to test widths as those code paths were not reachable and thus they're removed.