Bug 196058

Summary: Add an internal feature flag to disable the -webkit-overflow-scrolling CSS property
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ews-watchlist, koivisto, rniwa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 196803    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews107 for mac-highsierra-wk2
none
Patch
koivisto: review+
Patch
none
Patch none

Description Simon Fraser (smfr) 2019-03-20 23:07:43 PDT
Add an internal feature flag to disable the -webkit-overflow-scrolling CSS property
Comment 1 Simon Fraser (smfr) 2019-03-20 23:09:59 PDT
Created attachment 365492 [details]
Patch
Comment 2 Simon Fraser (smfr) 2019-03-20 23:10:02 PDT
<rdar://problem/49078202>
Comment 3 EWS Watchlist 2019-03-21 01:24:59 PDT
Comment on attachment 365492 [details]
Patch

Attachment 365492 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11594807

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 4 EWS Watchlist 2019-03-21 01:25:00 PDT
Created attachment 365517 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 5 Simon Fraser (smfr) 2019-03-21 08:55:46 PDT
Created attachment 365550 [details]
Patch
Comment 6 Antti Koivisto 2019-03-21 09:17:22 PDT
Comment on attachment 365550 [details]
Patch

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

r=me with change to const CSSParserContext&.

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:523
> +bool CSSParserFastPaths::isValidKeywordPropertyAndValue(CSSPropertyID propertyId, CSSValueID valueID, CSSParserMode parserMode, const CSSParserContext* context)

How would the caller know when to provide this context? Please make it a reference and like everywhere else it is used.

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:810
> +        WTFLogAlways("Parsing CSSPropertyWebkitOverflowScrolling; enabled %d", context && context->legacyOverflowScrollingTouchEnabled);

Please don't leave spammy logging in.
Comment 7 Simon Fraser (smfr) 2019-03-21 11:31:43 PDT
(In reply to Antti Koivisto from comment #6)
> Comment on attachment 365550 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365550&action=review
> 
> r=me with change to const CSSParserContext&.

There's a caller (inspector code) that doesn't have access to a CSSParserContext.
Comment 8 Antti Koivisto 2019-03-21 11:46:17 PDT
> There's a caller (inspector code) that doesn't have access to a
> CSSParserContext.

There is strictCSSParserContext() global for cases where a real context (with base urls etc) is not needed.
Comment 9 Simon Fraser (smfr) 2019-03-21 12:01:47 PDT
Created attachment 365583 [details]
Patch
Comment 10 Simon Fraser (smfr) 2019-03-21 12:48:25 PDT
Created attachment 365595 [details]
Patch
Comment 11 Simon Fraser (smfr) 2019-03-21 13:20:54 PDT
https://trac.webkit.org/r243318
Comment 12 Alex Christensen 2019-05-03 16:38:31 PDT
Comment on attachment 365595 [details]
Patch

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

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:811
> +            return nullptr;

return false
:(