[webkit-reviews] review denied: [Bug 121905] [GTK] LayoutTests/fast/images/exif-orientation.html is failing : [Attachment 212930] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Anton Obzhirov
<a.obzhirov at samsung.com>'s request for review:
Bug 121905: [GTK] LayoutTests/fast/images/exif-orientation.html is failing
https://bugs.webkit.org/show_bug.cgi?id=121905

Attachment 212930: Patch
https://bugs.webkit.org/attachment.cgi?id=212930&action=review

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
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?