| Summary: | clip-path: path() ignores page zooming (Command-+) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Noam Rosenthal <noam> | ||||||||
| Component: | CSS | Assignee: | 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
Noam Rosenthal
2020-10-05 07:20:59 PDT
Created attachment 410512 [details]
Very simple manual test case
Command-+ zooming, not pinch zooming. (In reply to Simon Fraser (smfr) from comment #3) > Command-+ zooming, not pinch zooming. Correct. Created attachment 410743 [details]
Patch
(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. 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. Created attachment 410762 [details]
Patch for landing
Committed r268138: <https://trac.webkit.org/changeset/268138> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410762 [details]. 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. (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? (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? (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 |