Bug 121905 – [GTK] LayoutTests/fast/images/exif-orientation.html is failing

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Anton Obzhirov]

I suspect we just need to enable RespectImageOrientation by default in WTR and DRT. Also for 
testing in MiniBrowser we should expose this flag in the settings as well.

[Anton Obzhirov]

Created attachment 212930 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 212930 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212930&action=review

Thanks.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1144
> +     /**
> +     * WebKitSettings:respect-image-orientation:

This is not correctly indented, the * should be lined up

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1149
> +     * Since: 2.3

This should be 2.4, the next stable release.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1156
> +            FALSE,

Why is it FALSE by default?

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2832
> + * Since: 2.3

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2848
> + * Since: 2.3

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h:397
> +webkit_settings_get_respect_image_orientation    (WebKitSettings *settings);

Parameters should be lines up with all other methods.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h:401
> +webkit_settings_set_respect_image_orientation    (WebKitSettings *settings,
> +                                                  gboolean        enabled);

Ditto.

> LayoutTests/ChangeLog:8
> +        Add correct test expected results.

I don't think this is affected by the change above, because WTR doesn't use WebKitSettings at all. It seems to be this is a rebaseline, so I would use different commits for this and the WebKitSettings API to avoid confusion.

> LayoutTests/platform/gtk/fast/images/exif-orientation-expected.txt:22
> -img5 size = 100px by 50px
> -img6 size = 100px by 50px
> -img7 size = 100px by 50px
> -img8 size = 100px by 50px
> +img5 size = 50px by 100px
> +img6 size = 50px by 100px
> +img7 size = 50px by 100px
> +img8 size = 50px by 100px

Do we still need a port specific expectation file?

[Anton Obzhirov]

Comment on attachment 212930 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212930&action=review

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1144
>> +     * WebKitSettings:respect-image-orientation:
> 
> This is not correctly indented, the * should be lined up

ok

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1149
>> +     * Since: 2.3
> 
> This should be 2.4, the next stable release.

ok

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1156
>> +            FALSE,
> 
> Why is it FALSE by default?

I guess you are right and it should be TRUE since we would want to see properly oriented images by default.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2832
>> + * Since: 2.3
> 
> Ditto.

ok

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2848
>> + * Since: 2.3
> 
> Ditto.

ok

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h:397
>> +webkit_settings_get_respect_image_orientation    (WebKitSettings *settings);
> 
> Parameters should be lines up with all other methods.

ok

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h:401
>> +                                                  gboolean        enabled);
> 
> Ditto.

ok

>> LayoutTests/ChangeLog:8
>> +        Add correct test expected results.
> 
> I don't think this is affected by the change above, because WTR doesn't use WebKitSettings at all. It seems to be this is a rebaseline, so I would use different commits for this and the WebKitSettings API to avoid confusion.

I'll create new bug for the WebKitSettings to track it.

>> LayoutTests/platform/gtk/fast/images/exif-orientation-expected.txt:22
>> +img8 size = 50px by 100px
> 
> Do we still need a port specific expectation file?

Probably not - I'll check.

[Anton Obzhirov]

Created attachment 213024 [details]
Patch

[Brady Eidson]

Comment on attachment 213024 [details]
Patch

Assuming that patches for review since 2013 are stale, r-

[Diego Pino]

There are no references to this bug in any TestExpectations. It's probable this bug was solved at some point but it wasn't marked as closed. I'm closing this bug now. If you think this bug report is still valid, please reopen it and add an entry to TestExpectations.