WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 91566
Integrate css3-images image-orientation with existing EXIF support
https://bugs.webkit.org/show_bug.cgi?id=91566
Summary
Integrate css3-images image-orientation with existing EXIF support
David Barr
Reported
2012-07-17 17:40:12 PDT
Add image-orientation support to WebKit.
http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-orientation
The css3-images module is at candidate recommendation. I propose to introduce the property initially behind a compile flag. There is existing code that allows a setting to determine whether embedded image orientations are respected. I propose that to integrate this new feature with the existing one, that if the setting is enabled and an embedded orientation is present that the embedded orientation take precedence over any out-of-band orientation.
Attachments
Draft patch
(23.64 KB, patch)
2012-07-22 20:58 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Draft patch
(41.54 KB, patch)
2012-07-24 23:49 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Draft patch
(17.11 KB, patch)
2012-07-25 22:47 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Draft patch
(17.38 KB, patch)
2012-07-25 23:13 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Updated patch
(42.29 KB, patch)
2013-07-19 03:56 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for ews
(35.15 KB, patch)
2013-08-21 22:17 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for ews
(36.85 KB, patch)
2013-08-21 23:47 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for ews
(39.48 KB, patch)
2013-08-22 06:27 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for ews
(37.56 KB, patch)
2013-08-26 01:04 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(37.55 KB, patch)
2013-08-26 01:56 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
(270.06 KB, application/zip)
2013-08-26 03:05 PDT
,
Build Bot
no flags
Details
Patch for mac port regression
(35.45 KB, patch)
2013-08-28 18:34 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
(822.79 KB, application/zip)
2013-08-28 20:03 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(754.13 KB, application/zip)
2013-08-28 20:30 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
(800.33 KB, application/zip)
2013-08-28 21:00 PDT
,
Build Bot
no flags
Details
Patch
(36.38 KB, patch)
2013-08-29 06:57 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
(682.56 KB, application/zip)
2013-08-29 08:41 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
(1.69 MB, application/zip)
2013-08-29 15:33 PDT
,
Build Bot
no flags
Details
Patch
(37.20 KB, patch)
2013-08-31 21:26 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(37.29 KB, patch)
2013-09-01 17:12 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(37.26 KB, patch)
2013-09-01 23:27 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Rebased patch
(37.32 KB, patch)
2013-09-12 19:10 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(37.32 KB, patch)
2013-09-13 02:58 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(37.32 KB, patch)
2013-09-13 03:32 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch according to Beth's comments
(37.48 KB, patch)
2013-09-19 03:37 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch according to Beth's comments
(38.00 KB, patch)
2013-09-19 08:42 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
(534.64 KB, application/zip)
2013-09-19 09:45 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
(529.88 KB, application/zip)
2013-09-19 12:31 PDT
,
Build Bot
no flags
Details
Patch
(44.64 KB, patch)
2013-09-20 22:21 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
(863.52 KB, application/zip)
2013-09-21 00:18 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
(884.02 KB, application/zip)
2013-09-21 01:47 PDT
,
Build Bot
no flags
Details
Patch
(42.72 KB, patch)
2013-10-06 06:16 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
(928.35 KB, application/zip)
2013-10-06 15:46 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
(885.87 KB, application/zip)
2013-10-06 16:27 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
(893.79 KB, application/zip)
2013-10-06 17:32 PDT
,
Build Bot
no flags
Details
Patch
(42.63 KB, patch)
2013-10-16 04:54 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(44.84 KB, patch)
2013-10-16 17:17 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(47.90 KB, patch)
2013-10-16 18:37 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Show Obsolete
(24)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2012-07-17 17:52:21 PDT
> I propose that to integrate this new feature with the existing one, that if the setting is enabled and an embedded orientation is present that the embedded orientation take precedence over any out-of-band orientation.
It's not clear to me that this is the correct behavior. css3-images dodges the issue of EXIF, but it seems that the spec implies that image-orientation would win.
David Barr
Comment 2
2012-07-17 18:43:55 PDT
From the spec: 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. From the somewhat-relate image-resolution section: from-image: The image's intrinsic resolution is taken as that specified by the image format. If the image does not specify its own resolution, the explicitly specified resolution is used (if given), else it defaults to '1ddpx'. My interpretation: The existing setting is considered non-conformant if enabled by default. How embedded image orientation and out-of-band orientation ought to be resolved is unspecified and left for a future level of the spec. However, if the future level follows the same pattern set forth for image-resolution, it would read: from-image: The image's intrinsic orientation is taken as that specified by the image format. If the image does not specify its own orientation, the explicitly specified orientation is used (if given), else it defaults to '0deg'. With this projection, I would consider the existing setting to be equivalent to: * { image-orientation: from-image; } In that case, the following ought to ignore embedded orientation: img { image-orientation: 90deg; } So I'm inclined to agree with Simon.
Mike Lawther
Comment 3
2012-07-17 20:08:35 PDT
I concur. If the page author has specified a rotation, that would win.
Tab Atkins Jr.
Comment 4
2012-07-17 23:55:27 PDT
I also agree. The setting that makes images respect EXIF can be recast as a user-agent-level rule that applies "image-orientation: from-image" to all image elements. The author's explicit declarations will then automatically win.
Tab Atkins Jr.
Comment 5
2012-07-17 23:56:52 PDT
Oh, and just to put your fear to rest, David, Images 4 (which I'll write up Any Day Now) will indeed have the "image-orientation: from-image" value, and it will work exactly like image-resolution, as you expect.
David Barr
Comment 6
2012-07-22 20:58:00 PDT
Created
attachment 153729
[details]
Draft patch Draft patch outlining scope of changes. Still navigating build and logical errors.
David Barr
Comment 7
2012-07-22 22:07:54 PDT
Comment on
attachment 153729
[details]
Draft patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153729&action=review
> Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:215 > + if (styleOrientation != OriginTopLeft && shouldRespectOrientation == RespectImageOrientation)
Oops, the condition should be: (styleOrientation == OriginTopLeft && shouldRespectOrientation == RespectImageOrientation). The basis for this condition is that we cannot distinguish between explicit 'image-orientation: 0deg' and the initial value.
David Barr
Comment 8
2012-07-24 23:49:45 PDT
Created
attachment 154275
[details]
Draft patch Largely complete, need to make sure it applies cleany to all ports and possibly split into smaller patches.
Early Warning System Bot
Comment 9
2012-07-25 00:16:40 PDT
Comment on
attachment 154275
[details]
Draft patch
Attachment 154275
[details]
did not pass qt-wk2-ews (qt): Output:
http://queues.webkit.org/results/13353123
Gyuyoung Kim
Comment 10
2012-07-25 00:17:49 PDT
Comment on
attachment 154275
[details]
Draft patch
Attachment 154275
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/13339161
Early Warning System Bot
Comment 11
2012-07-25 00:24:01 PDT
Comment on
attachment 154275
[details]
Draft patch
Attachment 154275
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/13347141
David Barr
Comment 12
2012-07-25 22:47:36 PDT
Created
attachment 154546
[details]
Draft patch Rebased against
bug 92330
.
David Barr
Comment 13
2012-07-25 23:13:01 PDT
Created
attachment 154550
[details]
Draft patch Missed a single instance of a variable rename.
Jeremy Apthorp
Comment 14
2012-11-03 20:02:11 PDT
David is no longer working on Chromium -- Mike, can you reassign?
Gyuyoung Kim
Comment 15
2013-07-19 03:56:07 PDT
Created
attachment 207077
[details]
Updated patch
Gyuyoung Kim
Comment 16
2013-07-19 03:56:57 PDT
Assigned to me.
Early Warning System Bot
Comment 17
2013-07-19 04:01:14 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1105789
Early Warning System Bot
Comment 18
2013-07-19 04:02:52 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/1098900
EFL EWS Bot
Comment 19
2013-07-19 04:03:41 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1099814
kov's GTK+ EWS bot
Comment 20
2013-07-19 04:04:12 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1132097
EFL EWS Bot
Comment 21
2013-07-19 04:11:10 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/1106832
Build Bot
Comment 22
2013-07-19 04:15:59 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1103803
Build Bot
Comment 23
2013-07-19 04:33:44 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1095774
Build Bot
Comment 24
2013-07-19 05:08:50 PDT
Comment on
attachment 207077
[details]
Updated patch
Attachment 207077
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/1131178
Gyuyoung Kim
Comment 25
2013-08-21 22:17:55 PDT
Created
attachment 209324
[details]
Patch for ews
Early Warning System Bot
Comment 26
2013-08-21 22:25:32 PDT
Comment on
attachment 209324
[details]
Patch for ews
Attachment 209324
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1523613
Early Warning System Bot
Comment 27
2013-08-21 22:26:26 PDT
Comment on
attachment 209324
[details]
Patch for ews
Attachment 209324
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/1532048
Build Bot
Comment 28
2013-08-21 22:28:26 PDT
Comment on
attachment 209324
[details]
Patch for ews
Attachment 209324
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1494354
Build Bot
Comment 29
2013-08-21 22:58:18 PDT
Comment on
attachment 209324
[details]
Patch for ews
Attachment 209324
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1524053
Gyuyoung Kim
Comment 30
2013-08-21 23:47:10 PDT
Created
attachment 209328
[details]
Patch for ews
Gyuyoung Kim
Comment 31
2013-08-22 06:27:19 PDT
Created
attachment 209360
[details]
Patch for ews
EFL EWS Bot
Comment 32
2013-08-22 06:44:49 PDT
Comment on
attachment 209360
[details]
Patch for ews
Attachment 209360
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/1538090
kov's GTK+ EWS bot
Comment 33
2013-08-22 06:51:20 PDT
Comment on
attachment 209360
[details]
Patch for ews
Attachment 209360
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1380553
Build Bot
Comment 34
2013-08-22 07:00:41 PDT
Comment on
attachment 209360
[details]
Patch for ews
Attachment 209360
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1488096
Build Bot
Comment 35
2013-08-22 07:11:03 PDT
Comment on
attachment 209360
[details]
Patch for ews
Attachment 209360
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1536098
EFL EWS Bot
Comment 36
2013-08-22 07:13:59 PDT
Comment on
attachment 209360
[details]
Patch for ews
Attachment 209360
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1393148
Gyuyoung Kim
Comment 37
2013-08-26 01:04:44 PDT
Created
attachment 209624
[details]
Patch for ews
Gyuyoung Kim
Comment 38
2013-08-26 01:56:06 PDT
Created
attachment 209628
[details]
Patch
Build Bot
Comment 39
2013-08-26 03:05:29 PDT
Comment on
attachment 209628
[details]
Patch
Attachment 209628
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1574003
New failing tests: fast/canvas/webgl/premultiplyalpha-test.html fast/canvas/pattern-with-transform.html fast/canvas/canvas-pattern-modify.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-canvas-rgb565.html fast/canvas/webgl/tex-sub-image-2d-bad-args.html canvas/philip/tests/initial.reset.pattern.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-canvas.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-canvas-rgba4444.html canvas/philip/tests/2d.pattern.modify.canvas2.html fast/canvas/webgl/texture-active-bind.html fast/canvas/webgl/tex-image-webgl.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-canvas-rgba5551.html fast/canvas/webgl/texture-npot.html fast/canvas/webgl/canvas-2d-webgl-texture.html canvas/philip/tests/2d.pattern.paint.orientation.canvas.html fast/canvas/canvas-pattern-behaviour.html canvas/philip/tests/2d.pattern.modify.canvas1.html canvas/philip/tests/2d.pattern.basic.canvas.html http/tests/inspector/inspect-element.html
Build Bot
Comment 40
2013-08-26 03:05:37 PDT
Created
attachment 209633
[details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Gyuyoung Kim
Comment 41
2013-08-26 03:53:41 PDT
Comment on
attachment 209628
[details]
Patch Need to fix some tests as well as exif-orientation tests on mac port.
Gyuyoung Kim
Comment 42
2013-08-28 18:34:57 PDT
Created
attachment 209941
[details]
Patch for mac port regression
kov's GTK+ EWS bot
Comment 43
2013-08-28 19:11:52 PDT
Comment on
attachment 209941
[details]
Patch for mac port regression
Attachment 209941
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1624444
Build Bot
Comment 44
2013-08-28 20:03:38 PDT
Comment on
attachment 209941
[details]
Patch for mac port regression
Attachment 209941
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1627425
New failing tests: fast/images/exif-orientation-image-document.html fast/images/exif-orientation.html fast/images/exif-orientation-composited.html
Build Bot
Comment 45
2013-08-28 20:03:47 PDT
Created
attachment 209948
[details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Build Bot
Comment 46
2013-08-28 20:30:10 PDT
Comment on
attachment 209941
[details]
Patch for mac port regression
Attachment 209941
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1635045
New failing tests: fast/images/exif-orientation-image-document.html fast/images/exif-orientation.html fast/images/exif-orientation-composited.html
Build Bot
Comment 47
2013-08-28 20:30:18 PDT
Created
attachment 209949
[details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Build Bot
Comment 48
2013-08-28 20:59:56 PDT
Comment on
attachment 209941
[details]
Patch for mac port regression
Attachment 209941
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1591772
New failing tests: fast/images/exif-orientation-image-document.html fast/images/exif-orientation.html fast/images/exif-orientation-composited.html
Build Bot
Comment 49
2013-08-28 21:00:04 PDT
Created
attachment 209950
[details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Gyuyoung Kim
Comment 50
2013-08-29 06:57:01 PDT
Created
attachment 209972
[details]
Patch
Build Bot
Comment 51
2013-08-29 08:40:53 PDT
Comment on
attachment 209972
[details]
Patch
Attachment 209972
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1618629
New failing tests: http/tests/security/canvas-cors-with-two-hosts.html fast/canvas/canvas-blending-image-over-gradient.html http/tests/security/canvas-remote-read-data-url-image.html fast/canvas/DrawImageSinglePixelStretch.html fast/images/paint-subrect-grid.html fast/canvas/canvas-blending-gradient-over-image.html http/tests/inspector/inspect-element.html fast/canvas/canvas-blending-image-over-image.html canvas/philip/tests/toDataURL.png.complexcolours.html fast/images/paint-subrect.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba5551.html canvas/philip/tests/toDataURL.jpeg.quality.basic.html fast/canvas/canvas-blending-color-over-image.html fast/dom/image-object.html http/tests/canvas/webgl/origin-clean-conformance.html canvas/philip/tests/toDataURL.jpeg.primarycolours.html http/tests/security/canvas-remote-read-remote-image-allowed-with-credentials.html fast/canvas/canvas-blending-pattern-over-image.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html canvas/philip/tests/toDataURL.png.primarycolours.html fast/canvas/canvas-blending-image-over-pattern.html canvas/philip/tests/security.dataURI.html fast/images/cmyk-jpeg-with-color-profile.html fast/canvas/canvas-incremental-repaint.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgb565.html fast/canvas/canvas-scale-drawImage-shadow.html fast/canvas/canvas-blending-image-over-color.html canvas/philip/tests/toDataURL.jpeg.alpha.html fast/canvas/canvas-drawImage-shadow.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba4444.html
Build Bot
Comment 52
2013-08-29 08:41:02 PDT
Created
attachment 209985
[details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Build Bot
Comment 53
2013-08-29 15:33:27 PDT
Comment on
attachment 209972
[details]
Patch
Attachment 209972
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1644081
New failing tests: http/tests/security/canvas-cors-with-two-hosts.html fast/canvas/canvas-blending-image-over-gradient.html http/tests/security/canvas-remote-read-data-url-image.html fast/canvas/DrawImageSinglePixelStretch.html fast/images/paint-subrect-grid.html fast/canvas/canvas-blending-gradient-over-image.html http/tests/inspector/inspect-element.html fast/canvas/canvas-blending-image-over-image.html canvas/philip/tests/toDataURL.png.complexcolours.html fast/images/paint-subrect.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba5551.html canvas/philip/tests/toDataURL.jpeg.quality.basic.html fast/canvas/canvas-blending-color-over-image.html fast/dom/image-object.html http/tests/canvas/webgl/origin-clean-conformance.html canvas/philip/tests/toDataURL.jpeg.primarycolours.html http/tests/security/canvas-remote-read-remote-image-allowed-with-credentials.html fast/canvas/canvas-blending-pattern-over-image.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html fast/canvas/canvas-blending-image-over-pattern.html platform/mac/fast/canvas/canvas-draw-xbm-image.html canvas/philip/tests/security.dataURI.html fast/images/cmyk-jpeg-with-color-profile.html platform/mac/editing/deleting/deletionUI-single-instance.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgb565.html fast/canvas/canvas-blending-image-over-color.html canvas/philip/tests/toDataURL.jpeg.alpha.html canvas/philip/tests/toDataURL.png.primarycolours.html fast/replaced/width-and-height-of-positioned-replaced-elements.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba4444.html
Build Bot
Comment 54
2013-08-29 15:33:36 PDT
Created
attachment 210038
[details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Gyuyoung Kim
Comment 55
2013-08-31 21:26:58 PDT
Created
attachment 210224
[details]
Patch
Early Warning System Bot
Comment 56
2013-08-31 21:35:49 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1657496
Early Warning System Bot
Comment 57
2013-08-31 21:37:38 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass qt-wk2-ews (qt-wk2): Output:
http://webkit-queues.appspot.com/results/1658539
EFL EWS Bot
Comment 58
2013-08-31 22:08:54 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/1661540
EFL EWS Bot
Comment 59
2013-08-31 22:11:33 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1680059
kov's GTK+ EWS bot
Comment 60
2013-08-31 23:21:27 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1603511
kov's GTK+ EWS bot
Comment 61
2013-08-31 23:28:51 PDT
Comment on
attachment 210224
[details]
Patch
Attachment 210224
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1659500
Gyuyoung Kim
Comment 62
2013-09-01 17:12:46 PDT
Created
attachment 210257
[details]
Patch
Gyuyoung Kim
Comment 63
2013-09-01 23:27:29 PDT
Created
attachment 210262
[details]
Patch
Gyuyoung Kim
Comment 64
2013-09-02 03:06:04 PDT
CC'ing Beth.
Gyuyoung Kim
Comment 65
2013-09-12 19:10:14 PDT
Created
attachment 211496
[details]
Rebased patch
Sergio Villar Senin
Comment 66
2013-09-13 02:48:38 PDT
Comment on
attachment 211496
[details]
Rebased patch View in context:
https://bugs.webkit.org/attachment.cgi?id=211496&action=review
LGTM
> Source/WebCore/loader/cache/CachedImage.cpp:260 > + ImageOrientationDescription orientationDescirption(renderer->shouldRespectImageOrientation());
nit orientationDescription
Gyuyoung Kim
Comment 67
2013-09-13 02:58:39 PDT
Created
attachment 211527
[details]
Patch
Gyuyoung Kim
Comment 68
2013-09-13 02:59:29 PDT
(In reply to
comment #66
)
> (From update of
attachment 211496
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=211496&action=review
> > LGTM > > > Source/WebCore/loader/cache/CachedImage.cpp:260 > > + ImageOrientationDescription orientationDescirption(renderer->shouldRespectImageOrientation()); > > nit orientationDescription
Thank you for your review. Fixed it.
kov's GTK+ EWS bot
Comment 69
2013-09-13 03:12:06 PDT
Comment on
attachment 211527
[details]
Patch
Attachment 211527
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1884153
Gyuyoung Kim
Comment 70
2013-09-13 03:32:03 PDT
Created
attachment 211530
[details]
Patch
Gyuyoung Kim
Comment 71
2013-09-15 21:19:40 PDT
Beth or others, could anyone review this patch ? I really want to land this patch.
Beth Dakin
Comment 72
2013-09-17 21:38:06 PDT
Comment on
attachment 211530
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=211530&action=review
> Source/WebCore/loader/cache/CachedImage.cpp:261 > + if (renderer && renderer->style())
It doesn't make any sense to null-check renderer here. If it is ever null, then you will crash on the previous line since you access it without a null check there. Figure out if it can ever be null, and if so, null-check it before you use it at all. If it cannot be null, then don't null check it.
> Source/WebCore/page/DragController.cpp:889 > + if (element->renderer() && element->renderer()->style())
Same comment about null-checking applies here.
> Source/WebCore/page/Frame.cpp:1040 > + if (renderer->style())
Is it necessary to null-check style?
> Source/WebCore/platform/graphics/Image.h:-206 > - virtual void draw(GraphicsContext*, const FloatRect& dstRect, const FloatRect& srcRect, ColorSpace styleColorSpace, CompositeOperator, BlendMode) = 0;
Why is this function being removed?
> Source/WebCore/rendering/RenderImage.cpp:150 > + if (diff == StyleDifferenceLayout
There's no reason to break this if-condition onto two lines.
> LayoutTests/fast/css/image-orientation/image-orientation.html:8 > +description('Apply image-orientation property and check computed style.');
You should add an explanation to the LayoutTest change log to explain your changes to this test. Was this description always inaccurate and you are just correcting it? Please explain.
Gyuyoung Kim
Comment 73
2013-09-19 03:27:56 PDT
Comment on
attachment 211530
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=211530&action=review
Beth, Thank you for your review :)
>> Source/WebCore/loader/cache/CachedImage.cpp:261 >> + if (renderer && renderer->style()) > > It doesn't make any sense to null-check renderer here. If it is ever null, then you will crash on the previous line since you access it without a null check there. Figure out if it can ever be null, and if so, null-check it before you use it at all. If it cannot be null, then don't null check it.
It looks we need to check if *renderer* is null. However, we don't need to check if style() is null. Renderer should have a style.
>> Source/WebCore/platform/graphics/Image.h:-206 >> - virtual void draw(GraphicsContext*, const FloatRect& dstRect, const FloatRect& srcRect, ColorSpace styleColorSpace, CompositeOperator, BlendMode) = 0; > > Why is this function being removed?
I thought that we don't need to keep this function anymore, because all child classes derived from Image class should implement this draw() which doesn't have *ImageOrientationDescription* argument additionally. But, I think now I don't have a clear reason to remove this function. So, I keep this function in next patch.
>> Source/WebCore/rendering/RenderImage.cpp:150 >> + if (diff == StyleDifferenceLayout > > There's no reason to break this if-condition onto two lines.
Done.
>> LayoutTests/fast/css/image-orientation/image-orientation.html:8 >> +description('Apply image-orientation property and check computed style.'); > > You should add an explanation to the LayoutTest change log to explain your changes to this test. Was this description always inaccurate and you are just correcting it? Please explain.
This line has wrong description for this test. This test is for testing image orientation, not image resolution. Fixed it.
> LayoutTests/fast/css/image-orientation/image-orientation.html:22 > + shouldBe('p.style.cssText', '"image-orientation: ' + test + ';"');
I think this test doesn't need to add a space at the end of line.
Gyuyoung Kim
Comment 74
2013-09-19 03:37:14 PDT
Created
attachment 212048
[details]
Patch according to Beth's comments
Gyuyoung Kim
Comment 75
2013-09-19 08:42:08 PDT
Created
attachment 212070
[details]
Patch according to Beth's comments
Build Bot
Comment 76
2013-09-19 09:45:43 PDT
Comment on
attachment 212070
[details]
Patch according to Beth's comments
Attachment 212070
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1968201
New failing tests: http/tests/security/canvas-cors-with-two-hosts.html fast/canvas/canvas-blending-image-over-pattern.html fast/canvas/canvas-blending-image-over-gradient.html http/tests/security/canvas-remote-read-data-url-image.html webgl/conformance/extensions/oes-texture-float-with-image.html fast/canvas/drawImage-with-invalid-args.html fast/canvas/canvas-blending-gradient-over-image.html fast/canvas/canvas-blending-image-over-image.html canvas/philip/tests/toDataURL.png.complexcolours.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba5551.html canvas/philip/tests/toDataURL.jpeg.quality.basic.html fast/canvas/canvas-blending-color-over-image.html fast/dom/image-object.html http/tests/canvas/webgl/origin-clean-conformance.html canvas/philip/tests/toDataURL.jpeg.primarycolours.html http/tests/security/canvas-remote-read-remote-image-allowed-with-credentials.html fast/canvas/canvas-blending-pattern-over-image.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html canvas/philip/tests/toDataURL.png.primarycolours.html svg/as-image/svg-canvas-xhtml-tainted.html canvas/philip/tests/security.dataURI.html svg/as-image/svg-canvas-link-not-colored.html fast/images/cmyk-jpeg-with-color-profile.html fast/canvas/canvas-incremental-repaint.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgb565.html fast/canvas/canvas-scale-drawImage-shadow.html fast/canvas/canvas-blending-image-over-color.html canvas/philip/tests/toDataURL.jpeg.alpha.html fast/canvas/canvas-drawImage-shadow.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba4444.html
Build Bot
Comment 77
2013-09-19 09:45:53 PDT
Created
attachment 212077
[details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 78
2013-09-19 12:31:27 PDT
Comment on
attachment 212070
[details]
Patch according to Beth's comments
Attachment 212070
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1999131
New failing tests: http/tests/security/canvas-cors-with-two-hosts.html webgl/conformance/canvas/to-data-url-test.html fast/canvas/canvas-blending-image-over-pattern.html fast/canvas/canvas-blending-image-over-gradient.html http/tests/security/canvas-remote-read-data-url-image.html webgl/conformance/extensions/oes-texture-float-with-image.html fast/canvas/drawImage-with-invalid-args.html fast/canvas/canvas-blending-gradient-over-image.html fast/canvas/canvas-blending-image-over-image.html canvas/philip/tests/toDataURL.png.complexcolours.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba5551.html canvas/philip/tests/toDataURL.jpeg.quality.basic.html fast/canvas/canvas-blending-color-over-image.html fast/dom/image-object.html http/tests/canvas/webgl/origin-clean-conformance.html canvas/philip/tests/toDataURL.jpeg.primarycolours.html fast/canvas/canvas-blending-pattern-over-image.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html canvas/philip/tests/toDataURL.png.primarycolours.html svg/as-image/svg-canvas-xhtml-tainted.html canvas/philip/tests/security.dataURI.html svg/as-image/svg-canvas-link-not-colored.html fast/images/cmyk-jpeg-with-color-profile.html fast/canvas/canvas-incremental-repaint.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgb565.html fast/canvas/canvas-scale-drawImage-shadow.html fast/canvas/canvas-blending-image-over-color.html canvas/philip/tests/toDataURL.jpeg.alpha.html fast/canvas/canvas-drawImage-shadow.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-image-rgba4444.html
Build Bot
Comment 79
2013-09-19 12:31:36 PDT
Created
attachment 212089
[details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Gyuyoung Kim
Comment 80
2013-09-20 22:21:34 PDT
Created
attachment 212254
[details]
Patch
kov's GTK+ EWS bot
Comment 81
2013-09-20 23:50:03 PDT
Comment on
attachment 212254
[details]
Patch
Attachment 212254
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1883503
Build Bot
Comment 82
2013-09-21 00:18:03 PDT
Comment on
attachment 212254
[details]
Patch
Attachment 212254
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1929206
New failing tests: http/tests/local/drag-over-remote-content.html editing/pasteboard/4947130.html platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html editing/pasteboard/drag-image-to-contenteditable-in-iframe.html http/tests/security/drag-over-remote-content-iframe.html editing/pasteboard/drag-and-drop-image-contenteditable.html editing/pasteboard/drag-selected-image-to-contenteditable.html editing/pasteboard/files-during-page-drags.html editing/selection/drag-to-contenteditable-iframe.html editing/pasteboard/drag-image-in-about-blank-frame.html fast/events/standalone-image-drag-to-editable.html http/tests/security/drag-drop-same-unique-origin.html http/tests/misc/bubble-drag-events.html
Build Bot
Comment 83
2013-09-21 00:18:12 PDT
Created
attachment 212259
[details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 84
2013-09-21 01:47:06 PDT
Comment on
attachment 212254
[details]
Patch
Attachment 212254
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/2029090
New failing tests: http/tests/local/drag-over-remote-content.html editing/pasteboard/4947130.html platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html editing/pasteboard/drag-image-to-contenteditable-in-iframe.html http/tests/security/drag-over-remote-content-iframe.html editing/pasteboard/drag-and-drop-image-contenteditable.html editing/pasteboard/drag-selected-image-to-contenteditable.html editing/pasteboard/files-during-page-drags.html editing/selection/drag-to-contenteditable-iframe.html editing/pasteboard/drag-image-in-about-blank-frame.html fast/events/standalone-image-drag-to-editable.html http/tests/security/drag-drop-same-unique-origin.html http/tests/misc/bubble-drag-events.html
Build Bot
Comment 85
2013-09-21 01:47:14 PDT
Created
attachment 212267
[details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Gyuyoung Kim
Comment 86
2013-10-06 06:16:29 PDT
Created
attachment 213510
[details]
Patch
Build Bot
Comment 87
2013-10-06 15:46:21 PDT
Comment on
attachment 213510
[details]
Patch
Attachment 213510
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/3493106
New failing tests: http/tests/local/drag-over-remote-content.html editing/pasteboard/4947130.html platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html editing/pasteboard/drag-image-to-contenteditable-in-iframe.html http/tests/security/drag-over-remote-content-iframe.html editing/pasteboard/drag-and-drop-image-contenteditable.html editing/pasteboard/drag-selected-image-to-contenteditable.html editing/pasteboard/files-during-page-drags.html editing/selection/drag-to-contenteditable-iframe.html editing/pasteboard/drag-image-in-about-blank-frame.html fast/events/standalone-image-drag-to-editable.html http/tests/security/drag-drop-same-unique-origin.html http/tests/misc/bubble-drag-events.html
Build Bot
Comment 88
2013-10-06 15:46:29 PDT
Created
attachment 213532
[details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 89
2013-10-06 16:27:28 PDT
Comment on
attachment 213510
[details]
Patch
Attachment 213510
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/3560013
New failing tests: http/tests/local/drag-over-remote-content.html editing/pasteboard/4947130.html platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html editing/pasteboard/drag-image-to-contenteditable-in-iframe.html http/tests/security/drag-over-remote-content-iframe.html editing/pasteboard/drag-and-drop-image-contenteditable.html editing/pasteboard/drag-selected-image-to-contenteditable.html editing/pasteboard/files-during-page-drags.html editing/selection/drag-to-contenteditable-iframe.html editing/pasteboard/drag-image-in-about-blank-frame.html fast/events/standalone-image-drag-to-editable.html http/tests/security/drag-drop-same-unique-origin.html http/tests/misc/bubble-drag-events.html
Build Bot
Comment 90
2013-10-06 16:27:35 PDT
Created
attachment 213537
[details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 91
2013-10-06 17:32:28 PDT
Comment on
attachment 213510
[details]
Patch
Attachment 213510
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/3573003
New failing tests: http/tests/local/drag-over-remote-content.html editing/pasteboard/4947130.html platform/mac/editing/pasteboard/drag-selections-to-contenteditable.html editing/pasteboard/drag-image-to-contenteditable-in-iframe.html http/tests/security/drag-over-remote-content-iframe.html editing/pasteboard/drag-and-drop-image-contenteditable.html editing/pasteboard/drag-selected-image-to-contenteditable.html editing/pasteboard/files-during-page-drags.html editing/selection/drag-to-contenteditable-iframe.html editing/pasteboard/drag-image-in-about-blank-frame.html fast/events/standalone-image-drag-to-editable.html http/tests/security/drag-drop-same-unique-origin.html http/tests/misc/bubble-drag-events.html
Build Bot
Comment 92
2013-10-06 17:32:35 PDT
Created
attachment 213543
[details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 93
2013-10-06 17:37:48 PDT
Comment on
attachment 213510
[details]
Patch
Attachment 213510
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/3493122
Gyuyoung Kim
Comment 94
2013-10-16 04:54:24 PDT
Created
attachment 214355
[details]
Patch
Build Bot
Comment 95
2013-10-16 06:30:32 PDT
Comment on
attachment 214355
[details]
Patch
Attachment 214355
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/4111259
Gyuyoung Kim
Comment 96
2013-10-16 17:17:06 PDT
Created
attachment 214408
[details]
Patch
Build Bot
Comment 97
2013-10-16 18:03:10 PDT
Comment on
attachment 214408
[details]
Patch
Attachment 214408
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/3494625
Gyuyoung Kim
Comment 98
2013-10-16 18:37:39 PDT
Created
attachment 214413
[details]
Patch
Gyuyoung Kim
Comment 99
2013-10-20 18:19:26 PDT
(In reply to
comment #72
)
> > Source/WebCore/platform/graphics/Image.h:-206 > > - virtual void draw(GraphicsContext*, const FloatRect& dstRect, const FloatRect& srcRect, ColorSpace styleColorSpace, CompositeOperator, BlendMode) = 0; > > Why is this function being removed?
Hi Beth, first of all, sorry for too late update. I fixed other stuffs according to your comments except for this draw() removal. In my humble opinion, this draw() has just called other draw() with an imageOrientationDescription argument. So, I don't think we don't need to keep it anymore. For instance, please look at below function. void BitmapImage::draw(GraphicsContext* ctx, const FloatRect& dstRect, const FloatRect& srcRect, ColorSpace styleColorSpace, CompositeOperator op, BlendMode blendMode) { draw(ctx, dstRect, srcRect, styleColorSpace, op, blendMode, ImageOrientationDescription()); }
WebKit Commit Bot
Comment 100
2013-10-23 20:38:29 PDT
Comment on
attachment 214413
[details]
Patch Clearing flags on attachment: 214413 Committed
r157909
: <
http://trac.webkit.org/changeset/157909
>
WebKit Commit Bot
Comment 101
2013-10-23 20:38:39 PDT
All reviewed patches have been landed. Closing bug.
Gyuyoung Kim
Comment 102
2013-10-23 22:38:03 PDT
30 regressions occur after landing this patch on GTK port only. Let me fix it. Please do not revert this patch.
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