Summary: | Implement css3-images image-orientation | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Barr <davidbarr> | ||||||||||||||||||||||||||||||||||
Component: | CSS | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | 50167214, 7raivis, antonio, ap, benjamin, bfulgham, cdumez, claude.pache, cmarcelo, commit-queue, david-webkit, dbates, dpaddock, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, gyuyoung.kim, jackalmage, james, jeremya, joepeck, keith_miller, kondapallykalyan, macpherson, marc-webkit, mark.lam, menard, mikelawther, msaboff, noel.gordon, pdr, rniwa, saam, sabouhallawa, simon.fraser, syoichi, tabatkins, thakis, thorton, tzagallo, webkit-bug-importer, zcorpan | ||||||||||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar, WebExposed | ||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||||
URL: | http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-orientation | ||||||||||||||||||||||||||||||||||||
Bug Depends on: | 89055, 89624, 91566, 91961, 93422, 200553, 201123, 203355 | ||||||||||||||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
David Barr
2012-06-13 19:07:42 PDT
Advertised on webkit-dev: http://thread.gmane.org/gmane.os.opendarwin.webkit.devel/21130 I wonder why this property doesn't have a value to auto-rotate based on EXIF data. This property purposefully ignores EXIF, which I think is stupid: "Note that some devices will "tag" an image with some metadata indicating its correct orientation, so image viewing software can do the necessary transformation themselves. Due to legacy compatibility restraints, Web browsers are required to ignore this data by default. A future level of this specification is expected to have a value that applies the metadata-specified transformation automatically." I think images with EXIF data are far more common than those encoded sideways It's also stupid that the spec doesn't use the term "EXIF", which is what most people would search for. David: do you have any data about how common is the issue that this property is trying to address? No hard data on this, other than searching online and finding devleopers asking on forums (eg http://stackoverflow.com/questions/1526448/rotating-an-image-using-css). This property affects the intrinsic height/width of the image, so if eg you wanted to display a bunch of photos in a gallery and have them all portrait oriented, this would allow you to do that as well as having the layout work. There's a thread at <http://lists.w3.org/Archives/Public/www-style/2012Feb/0311.html> Tab says "The spec inherited this property from existing print media stuff". I don't think this property is particularly relevant to most webkit clients. There are lots of far more pressing things to work on. If you care about image orientation, it would make far more sense to solve the EXIF problem. CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to. Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening? (In reply to comment #7) > CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to. > > Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening? Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something. Fantasai and I plan to start seriously working on Images 4 soonish. I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later. (In reply to comment #8) > (In reply to comment #7) > > CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to. > > > > Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening? > > Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something. Fantasai and I plan to start seriously working on Images 4 soonish. Will can one feature detect between a level 3 implementation and a level 4 implementation? I guess you have to set image-orientation with a level 4 value and see if you can read it back out? > I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later. If Images 4 is starting soonish, maybe it's better to just wait until it's underway. Bug 19688 is about respecting EXIF. (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to. > > > > > > Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening? > > > > Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something. Fantasai and I plan to start seriously working on Images 4 soonish. > > Will can one feature detect between a level 3 implementation and a level 4 implementation? I guess you have to set image-orientation with a level 4 value and see if you can read it back out? Yup, that's the standard way to detect new values for an existing property. (In the future, you'll be able to do this more directly. In CSS, you'll have the @supports rule, and I imagine there will be a related CSSOM function.) > > I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later. > > If Images 4 is starting soonish, maybe it's better to just wait until it's underway. The property isn't super important yet, but it doesn't hurt to work on it now, and it would be nice to have a passing browser implementation of the tests that I'll be writing soon. David is no longer working on Chromium -- Mike, can you reassign? Still nothing on that feature ? It's in Firefox since ages : http://sethfowler.org/blog/2013/09/13/new-in-firefox-26-css-image-orientation/ see also chromium issue: https://code.google.com/p/chromium/issues/detail?id=158753 Thanks in advance ! Marc (In reply to comment #13) > Still nothing on that feature ? > > It's in Firefox since ages : > http://sethfowler.org/blog/2013/09/13/new-in-firefox-26-css-image- > orientation/ > > see also chromium issue: > https://code.google.com/p/chromium/issues/detail?id=158753 > > Thanks in advance ! > Marc Hi, current EFL and GTK ports have enabled this feature. Nobody to fix it? (In reply to comment #15) > Nobody to fix it? What bug or feature do you want to fix ? In my view, the thing that would really help in general would be support for 'image-orientation: from-image'. That would at least provide a way to be confident images are rendered with the same orientation in iOS and desktop WebKit. please support the image-orientation from-image feature. On websites like this https://www.fabsurplus.com/sdi_catalog/salesItemDetails.do?id=87930 with user generated content images, orientating the pictures correctly would require development work in order to show correctly, while they look correctly already in a browser like Mozilla Firefox that already implements this CSS3 feature that is indeed very handy in a case like this, with just the use of the image-orientation : from-image handle in CSS. Thanks! The current draft specs is: https://drafts.csswg.org/css-images-4/#image-orientation It states: "If the image has an orientation specified in its metadata, such as EXIF, the UA must rotate or flip the image to correctly orient it as the metadata specifies." The MDN claims also the image-orientation property is likely to be deprecated: https://developer.mozilla.org/en-US/docs/Web/CSS/image-orientation. The currentWebKit implementation of the image orientation is confusing. The image orientation is respected only when the image is opened as an ImageDocument, i.e. standalone image. When the image is embedded in a page as an <img> element or as a css background image, the image orientation is ignored. See RenderElement::shouldRespectImageOrientation(). I think it is time to automatically respect the image orientation specified in the image’s metadata. Created attachment 375375 [details]
Patch
Comment on attachment 375375 [details] Patch Attachment 375375 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12849555 New failing tests: http/tests/images/draw-pattern-slow-load-large-image.html Created attachment 375380 [details]
Archive of layout-test-results from ews103 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 375375 [details] Patch Attachment 375375 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12849893 Number of test failures exceeded the failure limit. Created attachment 375390 [details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 375423 [details]
Patch
Created attachment 375428 [details]
Patch
Comment on attachment 375428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375428&action=review Can we break this up so it's not such a big rewrite of the existing code? > Source/WebCore/css/parser/CSSPropertyParser.cpp:2902 > + if (range.peek().id() == CSSValueAuto || range.peek().id() == CSSValueFromImage) { There's no 'auto' in the draft spec. Created attachment 375433 [details]
Patch
Comment on attachment 375428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375428&action=review >> Source/WebCore/css/parser/CSSPropertyParser.cpp:2902 >> + if (range.peek().id() == CSSValueAuto || range.peek().id() == CSSValueFromImage) { > > There's no 'auto' in the draft spec. Yes this is right. I meant to use CSSValueNone. See StyleBuilderConverter::convertImageOrientation(). There, CSSValueNone is used and not CSSValueAuto. Comment on attachment 375433 [details] Patch Attachment 375433 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12853466 New failing tests: http/tests/images/draw-pattern-slow-load-large-image.html Created attachment 375441 [details]
Archive of layout-test-results from ews101 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 375433 [details] Patch Attachment 375433 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12853511 Number of test failures exceeded the failure limit. Created attachment 375445 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
The CSSWG has changed the initial value of image-orientation to "from-image" (aka respect the EXIF orientation by default). https://github.com/whatwg/html/issues/4495 Sorry, wrong link. https://github.com/w3c/csswg-drafts/issues/3799 (In reply to Simon Pieters from comment #37) > The CSSWG has changed the initial value of image-orientation to "from-image" > (aka respect the EXIF orientation by default). > > https://github.com/whatwg/html/issues/4495 Actually the current proposal is to remove the property entirely, and just have the normal behavior be to respect EXIF. OK, is that plan in https://github.com/w3c/csswg-drafts/issues/4165#issuecomment-519304078 or somewhere else? Created attachment 376706 [details]
Patch
Comment on attachment 376706 [details] Patch Attachment 376706 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12942568 New failing tests: http/tests/images/draw-pattern-slow-load-large-image.html Created attachment 376710 [details]
Archive of layout-test-results from ews100 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 376706 [details] Patch Attachment 376706 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12942597 New failing tests: http/tests/images/draw-pattern-slow-load-large-image.html Created attachment 376714 [details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 376729 [details]
Patch
It seems the CSSWG resolution was noted in https://github.com/w3c/csswg-drafts/issues/4164 , though the issue is still open and discussion is ongoing. Created attachment 377035 [details]
Patch
Comment on attachment 377035 [details]
Patch
I'm going to make a prediction and say that CSS WG will end up bringing back image-orientation with "none" and "from-image". Given that prediction, I think the most sensible approach right now is to leave everything behind the #ifdef, but just change the default macOS behavior.
Created attachment 386994 [details]
Patch
Comment on attachment 386994 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386994&action=review > Source/JavaScriptCore/ChangeLog:8 > + * Configurations/FeatureDefines.xcconfig: You should say what the change is here. > Source/WTF/ChangeLog:8 > + * wtf/FeatureDefines.h: Ditto. > Source/WebCore/rendering/style/RenderStyle.h:2112 > + if (m_rareInheritedData->imageOrientation == ImageOrientation::FromImage) Can this function just return m_rareInheritedData->imageOrientation ? Created attachment 387027 [details]
Patch
Created attachment 387031 [details]
Patch
Comment on attachment 387031 [details] Patch Clearing flags on attachment: 387031 Committed r254187: <https://trac.webkit.org/changeset/254187> All reviewed patches have been landed. Closing bug. Cool. Any tests planned for drawing an EXIF image to a <canvas> element? (In reply to noel gordon from comment #56) > Cool. Any tests planned for drawing an EXIF image to a <canvas> element? When drawing images on a <canvas> element the EXIF image orientation is always respected. The test LayoutTests/fast/images/exif-orientation-canvas.html covers this case. (In reply to Said Abou-Hallawa from comment #57) > The test > LayoutTests/fast/images/exif-orientation-canvas.html covers this case. Is there a reason the tests here were not upstreamed to web-platform-tests? |