The EXIF orientation should be respected when rendering images in the following cases: 1. The image is the src of an <img> element 2. The image is the css background image of an HTML element 3. The image is the src of an SVG <image> element 4. The image is the src of an SVG <feImage> element 5. Drawing the image on a canvas 6. The image is the css content of an HTML element 7. The image is an ImageDocument 8. The image is data of an <object> element This should be the behavior on all ports till the css image-orientation specs is finalized.
Created attachment 377221 [details] Patch
<rdar://problem/54728386>
Created attachment 377346 [details] Patch
Created attachment 377362 [details] Patch
Created attachment 377428 [details] Patch
Created attachment 377444 [details] Patch
Created attachment 377454 [details] Patch for review
Created attachment 377493 [details] Patch
Created attachment 377502 [details] Patch
Comment on attachment 377502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377502&action=review > LayoutTests/fast/images/exif-orientation-composited.html:-3 > -<html> > -<head> > -<script> This is an important test to keep. It tests the directly-composited image code path (see RenderLayerBacking::isDirectlyCompositedImage()).
Created attachment 377807 [details] Patch
Comment on attachment 377807 [details] Patch Clearing flags on attachment: 377807 Committed r249364: <https://trac.webkit.org/changeset/249364>
All reviewed patches have been landed. Closing bug.
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 377502 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377502&action=review > > > LayoutTests/fast/images/exif-orientation-composited.html:-3 > > -<html> > > -<head> > > -<script> > > This is an important test to keep. It tests the directly-composited image > code path (see RenderLayerBacking::isDirectlyCompositedImage()).
Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review Very nice set of tests, and thank you for adding them. Couple of comments ... > LayoutTests/fast/images/exif-orientation-background-expected.html:11 > + width: 102; Maybe 102px here? Noticed px missing here and in other tests. > LayoutTests/fast/images/exif-orientation-composited.html:20 > + -webkit-transform: translateZ(0); > + border: 1px solid black; Last I looked at webkit code in detail (a few years back), having CSS border style on a image disallowed direct compositing of the image. Maybe border style no longer does that?
(In reply to noel gordon from comment #15) > Comment on attachment 377807 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377807&action=review > > Very nice set of tests, and thank you for adding them. Couple of comments > ... > > > LayoutTests/fast/images/exif-orientation-background-expected.html:11 > > + width: 102; > > Maybe 102px here? Noticed px missing here and in other tests. > > > LayoutTests/fast/images/exif-orientation-composited.html:20 > > + -webkit-transform: translateZ(0); > > + border: 1px solid black; > > Last I looked at webkit code in detail (a few years back), having CSS border > style on a image disallowed direct compositing of the image. Maybe border > style no longer does that? You're right; borders will kick you out of the directly composited image code path.
https://bugs.webkit.org/show_bug.cgi?id=201432 addresses the last review comments.
Thanks for checking, Said.
Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review > Source/WebCore/rendering/RenderElement.cpp:-2063 > - if (settings().shouldRespectImageOrientation() && is<HTMLImageElement>(element())) If I'm not wrong this was the only usage of shouldRespectImageOrientation(). After this change do we still need "shouldRespectImageOrientation" flag defined at WebCore/page/Settings.yaml?
Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review >> Source/WebCore/rendering/RenderElement.cpp:-2063 >> - if (settings().shouldRespectImageOrientation() && is<HTMLImageElement>(element())) > > If I'm not wrong this was the only usage of shouldRespectImageOrientation(). > After this change do we still need "shouldRespectImageOrientation" flag defined at WebCore/page/Settings.yaml? No. We do not really need it. I kept the shouldRespectImageOrientation setting because the preference key WebKitShouldRespectImageOrientation is still using it. But surely we need to clean up this area.