Bug 217311

Summary: clip-path: path() ignores page zooming (Command-+)
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: CSSAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, clopez, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, sabouhallawa, simon.fraser, webkit-bug-importer, youennf, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=224795
Bug Depends on:    
Bug Blocks: 126207    
Attachments:
Description Flags
Very simple manual test case
none
Patch
none
Patch for landing none

Noam Rosenthal
Reported 2020-10-05 07:20:59 PDT
When zooming the page, the content gets zoomed but the clip-path doesn't. See w3c test: https://github.com/web-platform-tests/wpt/pull/25981
Attachments
Very simple manual test case (174 bytes, text/html)
2020-10-05 07:26 PDT, Noam Rosenthal 		no flags
Details
Patch (197.88 KB, patch)
2020-10-07 04:04 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Patch for landing (198.29 KB, patch)
2020-10-07 11:10 PDT, Noam Rosenthal 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Noam Rosenthal
Comment 1 2020-10-05 07:26:46 PDT
Created attachment 410512 [details] Very simple manual test case
Radar WebKit Bug Importer
Comment 2 2020-10-05 17:01:11 PDT
<rdar://problem/69977271>
Simon Fraser (smfr)
Comment 3 2020-10-05 17:06:31 PDT
Command-+ zooming, not pinch zooming.
Noam Rosenthal
Comment 4 2020-10-05 23:07:23 PDT
(In reply to Simon Fraser (smfr) from comment #3) > Command-+ zooming, not pinch zooming. Correct.
Noam Rosenthal
Comment 5 2020-10-07 04:04:17 PDT
Created attachment 410743 [details] Patch
Noam Rosenthal
Comment 6 2020-10-07 04:53:41 PDT
(In reply to Noam Rosenthal from comment #5) > Created attachment 410743 [details] > Patch The patch is big because it also includes importing a lot of w3c tests.
Darin Adler
Comment 7 2020-10-07 08:21:45 PDT
Comment on attachment 410743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410743&action=review > Source/WebCore/rendering/style/BasicShapes.cpp:64 > +struct SVGPathTransformedByteStream { Not new to this patch: this struct should not use m_ prefixes on its public data members. > Source/WebCore/rendering/style/BasicShapes.cpp:65 > + SVGPathTransformedByteStream(const FloatPoint& offset, float zoom, const SVGPathByteStream& rawStream) Should be able to use aggregate initialization instead of a constructor. > Source/WebCore/rendering/style/BasicShapes.cpp:123 > +struct TransformedByteStreamPathPolicy : public TinyLRUCachePolicy<SVGPathTransformedByteStream, Path> { No need for public here > Source/WebCore/rendering/style/BasicShapes.cpp:124 > public: No need for public here > LayoutTests/imported/w3c/ChangeLog:10 > + Some of them don't pass yet for unrelated reasons, skipped in TestExpectations. Expect failure, not skipped. Which is good/better.
Noam Rosenthal
Comment 8 2020-10-07 11:10:18 PDT
Created attachment 410762 [details] Patch for landing
EWS
Comment 9 2020-10-07 11:54:32 PDT
Committed r268138: <https://trac.webkit.org/changeset/268138> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410762 [details].
Simon Fraser (smfr)
Comment 10 2021-08-28 12:14:04 PDT
Comment on attachment 410762 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=410762&action=review > LayoutTests/imported/w3c/web-platform-tests/css/css-masking/clip-path/clip-path-path-with-zoom.html:22 > + zoom: 2; zoom is not a standardized property, and is not supported by Firefox so I don't think it's appropriate to use it in WPT tests.
Noam Rosenthal
Comment 11 2021-08-28 13:23:38 PDT
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 410762 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=410762&action=review > > > LayoutTests/imported/w3c/web-platform-tests/css/css-masking/clip-path/clip-path-path-with-zoom.html:22 > > + zoom: 2; > > zoom is not a standardized property, and is not supported by Firefox so I > don't think it's appropriate to use it in WPT tests. there is no current way to represent page zoom in Selenium/WPT. There is an open issue for it here: https://github.com/w3c/webdriver/issues/1378 So the zoom property here is not used as a standard presentation property, but rather as a way to simulate a user-initiated page zoom. What would you suggest to do instead?
Simon Fraser (smfr)
Comment 12 2021-08-28 13:58:16 PDT
(In reply to Noam Rosenthal from comment #11) > (In reply to Simon Fraser (smfr) from comment #10) > > Comment on attachment 410762 [details] > > Patch for landing > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=410762&action=review > > > > > LayoutTests/imported/w3c/web-platform-tests/css/css-masking/clip-path/clip-path-path-with-zoom.html:22 > > > + zoom: 2; > > > > zoom is not a standardized property, and is not supported by Firefox so I > > don't think it's appropriate to use it in WPT tests. > > there is no current way to represent page zoom in Selenium/WPT. There is an > open issue for it here: https://github.com/w3c/webdriver/issues/1378 > > So the zoom property here is not used as a standard presentation property, > but rather as a way to simulate a user-initiated page zoom. What would you > suggest to do instead? Move the tests out of WPT so they don't fail in Firefox?
Noam Rosenthal
Comment 13 2021-08-28 14:25:05 PDT
(In reply to Simon Fraser (smfr) from comment #12) > (In reply to Noam Rosenthal from comment #11) > > (In reply to Simon Fraser (smfr) from comment #10) > > > Comment on attachment 410762 [details] > > > Patch for landing > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=410762&action=review > > > > > > > LayoutTests/imported/w3c/web-platform-tests/css/css-masking/clip-path/clip-path-path-with-zoom.html:22 > > > > + zoom: 2; > > > > > > zoom is not a standardized property, and is not supported by Firefox so I > > > don't think it's appropriate to use it in WPT tests. > > > > there is no current way to represent page zoom in Selenium/WPT. There is an > > open issue for it here: https://github.com/w3c/webdriver/issues/1378 > > > > So the zoom property here is not used as a standard presentation property, > > but rather as a way to simulate a user-initiated page zoom. What would you > > suggest to do instead? > > Move the tests out of WPT so they don't fail in Firefox? Doable Not sure when I can get to it myself though but can try in a few weeks
Note You need to log in before you can comment on or make changes to this bug.