Summary: | [CSS] Enable css-image-orientation on EFL and GTK ports. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Component: | CSS | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bdakin, commit-queue, dbates, gtk-ews, japhet, mrobinson, philn, rego+ews, sergio, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2013-11-03 00:47:48 PDT
Created attachment 215850 [details]
Patch
Created attachment 215868 [details]
Patch
Comment on attachment 215868 [details] Patch Attachment 215868 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/20398067 (In reply to comment #3) > (From update of attachment 215868 [details]) > Attachment 215868 [details] did not pass gtk-ews (gtk): > Output: http://webkit-queues.appspot.com/results/20398067 When I build gtk port with this patch, there is no build break. It seems to me gtk ews doesn't generate *CSSPropertyImageOrientation* with this patch. Is this right ? Created attachment 215888 [details]
Patch
Comment on attachment 215888 [details] Patch Attachment 215888 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/19648639 CC'ing Beth and Martin, Beth, could you take a look this ? Martin, in gtk build error cases, there is no build break when I build gtk port using this patch. It looks gtk ews doesn't support to build gtk port from scratch. Could you take a look this ? Comment on attachment 215888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215888&action=review > Source/WebCore/ChangeLog:9 > + when enabling css-image-orientaiton. This fixes those wrong implementation as well as enables it on EFL nit: enabling css-image-orientation. > Source/WebCore/loader/cache/CachedImage.cpp:266 > + imageSize = static_cast<BitmapImage*>(m_image.get())->sizeRespectingOrientation(orientationDescription); There's still a problem here, as we don't set imageSize if we don't get to this inner if, and hence get a zero sized image, since we won't fall in the outer else clause. I suggest you initialize imageSize in its declaration with m_image->size() and remove the outer else clause, to make sure we always have imageSize with a valid value. If needed, it will be changed accordingly inside ENABLE(CSS_IMAGE_ORIENTATION) / ENABLE(SVG). Created attachment 216009 [details]
Patch
(In reply to comment #8) > (From update of attachment 215888 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=215888&action=review > > > Source/WebCore/ChangeLog:9 > > + when enabling css-image-orientaiton. This fixes those wrong implementation as well as enables it on EFL > > nit: enabling css-image-orientation. Fixed. Thanks. > > Source/WebCore/loader/cache/CachedImage.cpp:266 > > + imageSize = static_cast<BitmapImage*>(m_image.get())->sizeRespectingOrientation(orientationDescription); > > There's still a problem here, as we don't set imageSize if we don't get to this inner if, and hence get a zero sized image, since we won't fall in the outer else clause. I suggest you initialize imageSize in its declaration with m_image->size() and remove the outer else clause, to make sure we always have imageSize with a valid value. Yes, we shouldn't make any situation that *imageSize* is not initialized. Fixed. Comment on attachment 216009 [details] Patch Attachment 216009 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/19568955 Comment on attachment 216009 [details] Patch Clearing flags on attachment: 216009 Committed r158659: <http://trac.webkit.org/changeset/158659> All reviewed patches have been landed. Closing bug. *** Bug 123256 has been marked as a duplicate of this bug. *** |