WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
217294
CSS image-orientation: none should be ignored for cross-origin images
https://bugs.webkit.org/show_bug.cgi?id=217294
Summary
CSS image-orientation: none should be ignored for cross-origin images
Noam Rosenthal
Reported
2020-10-04 07:13:39 PDT
See
https://github.com/w3c/csswg-drafts/issues/5165
To avoid leaking metadata to the embedder, images that can't be accessed by the document's security origin should always render with their default (from-image) orientation.
Attachments
Patch
(23.84 KB, patch)
2020-10-04 23:05 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch
(23.80 KB, patch)
2020-10-06 03:02 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch
(23.85 KB, patch)
2020-10-06 03:09 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch
(24.51 KB, patch)
2020-10-08 06:00 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch
(24.53 KB, patch)
2020-10-08 08:44 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch
(21.37 KB, patch)
2020-10-08 10:49 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch for landing
(20.91 KB, patch)
2020-10-09 00:36 PDT
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Noam Rosenthal
Comment 1
2020-10-04 07:17:56 PDT
Equivalent chromium bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=1110330
Noam Rosenthal
Comment 2
2020-10-04 23:05:33 PDT
Created
attachment 410502
[details]
Patch
EWS Watchlist
Comment 3
2020-10-04 23:06:32 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see
https://trac.webkit.org/wiki/WPTExportProcess
Noam Rosenthal
Comment 4
2020-10-06 03:02:15 PDT
Created
attachment 410622
[details]
Patch
Noam Rosenthal
Comment 5
2020-10-06 03:09:59 PDT
Created
attachment 410623
[details]
Patch
Noam Rosenthal
Comment 6
2020-10-06 10:06:04 PDT
The iOS layout test failure is a slight image-diff... I think the patch can be reviewed.
youenn fablet
Comment 7
2020-10-08 01:16:45 PDT
Comment on
attachment 410623
[details]
Patch iOS bot fails http/tests/security/canvas-remote-image-orientation.html. View in context:
https://bugs.webkit.org/attachment.cgi?id=410623&action=review
> Source/WebCore/html/HTMLImageElement.h:143 > + bool allowsOrientationOverride() const override;
final?
> Source/WebCore/rendering/RenderElement.cpp:2151 > + return orientation;
Why not: return element() && !element()->allowsOrientationOverride() ? ImageOrientation::FromImage : style().imageOrientation();
Noam Rosenthal
Comment 8
2020-10-08 06:00:33 PDT
Created
attachment 410831
[details]
Patch
Noam Rosenthal
Comment 9
2020-10-08 08:44:20 PDT
Created
attachment 410847
[details]
Patch
youenn fablet
Comment 10
2020-10-08 08:59:24 PDT
Comment on
attachment 410847
[details]
Patch LGTM overall but I hope we can change http/tests/security/canvas-remote-image-orientation.html so that it passes on iOS. View in context:
https://bugs.webkit.org/attachment.cgi?id=410847&action=review
> Source/WebCore/ChangeLog:31 > + Only apply orientation for eligible images.
Strange indentation here and above.
> Source/WebCore/html/HTMLImageElement.cpp:690 > + return image->isOriginClean(&(document().securityOrigin()));
Could be a one liner too.
> LayoutTests/ChangeLog:11 > + Added a test to ensure image-orientation: none is not respected for remote images in canvas
Strange indentation.
> LayoutTests/imported/w3c/ChangeLog:10 > + Imported a new W3C test for remote image with image-orientation.
Strange indentation.
> LayoutTests/platform/ios-simulator-wk2/TestExpectations:11 > +http/tests/security/canvas-remote-image-orientation.html [ ImageOnlyFailure ]
Isn't there a way we could change the test slightly? It seems the WPT tests are passing, so maybe the idea is to change the canvas size? Also, we could migrate that test to http/wpt so that we would more easily reuse WPT exif-orientation-3-lr.jpg (or even contribute it to WPT at some point).
Noam Rosenthal
Comment 11
2020-10-08 09:07:27 PDT
Comment on
attachment 410847
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=410847&action=review
>> LayoutTests/platform/ios-simulator-wk2/TestExpectations:11 >> +http/tests/security/canvas-remote-image-orientation.html [ ImageOnlyFailure ] > > Isn't there a way we could change the test slightly? > It seems the WPT tests are passing, so maybe the idea is to change the canvas size? > > Also, we could migrate that test to http/wpt so that we would more easily reuse WPT exif-orientation-3-lr.jpg (or even contribute it to WPT at some point).
There is already a wpt test for this. I wasn't aware of http/wpt, I'll import it there instead of the webkit-specific test. Should I land this with the test working on ios, or do you want to re-review once I have that ready?
youenn fablet
Comment 12
2020-10-08 09:08:53 PDT
> There is already a wpt test for this. I wasn't aware of http/wpt, I'll > import it there instead of the webkit-specific test. Should I land this with > the test working on ios, or do you want to re-review once I have that ready?
I think the patch is good to land once the test works on iOS.
Noam Rosenthal
Comment 13
2020-10-08 10:49:50 PDT
Created
attachment 410861
[details]
Patch
Said Abou-Hallawa
Comment 14
2020-10-08 11:25:35 PDT
Comment on
attachment 410861
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=410861&action=review
> Source/WebCore/dom/Element.h:340 > + virtual bool allowsOrientationOverride() const { return true; }
I do not think it is worthy adding this as a virtual method because it is overridden by one superclass which is HTMLImageElement. All it does is it saves you a casting to HTMLImageElement in RenderElement::imageOrientation().
> Source/WebCore/rendering/RenderElement.cpp:2143 > + return (element() && !element()->allowsOrientationOverride()) ? ImageOrientation(ImageOrientation::FromImage) : style().imageOrientation();
Can be written like this: auto* imageElement = is<HTMLImageElement>(element()) ? downcast<HTMLImageElement>(element()) : nullptr; return (imageElement && ...
Noam Rosenthal
Comment 15
2020-10-09 00:36:04 PDT
Created
attachment 410920
[details]
Patch for landing
EWS
Comment 16
2020-10-09 01:19:05 PDT
Committed
r268249
: <
https://trac.webkit.org/changeset/268249
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 410920
[details]
.
Radar WebKit Bug Importer
Comment 17
2020-10-09 01:20:21 PDT
<
rdar://problem/70130571
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug