Bug 55142 – Make it possible to test the targetdensity-dpi support

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Kenneth Rohde Christiansen]

SSIA

[Kenneth Rohde Christiansen]

Created attachment 83648 [details]
Patch

[WebKit Review Bot]

Attachment 83648 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:61:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 134 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 83648 [details]
Patch

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

r=me with moans:

> Source/WebKit/qt/ChangeLog:9
> +        Test the viewport meta tag feature targetdensity-dpi by
> +        adding a new extra arguments to dumpConfigurationForViewpor

<kenneth repeat="all ChangeLogs">
...adding extra arguments...
Typo, dumpConfigurationForViewport
</kenneth>

> Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.h:187
> -    static QString viewportAsText(QWebPage*, const QSize&);
> +    static QString viewportAsText(QWebPage*, int, const QSize&, const QSize&);

Variable names would be useful here.

> Tools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:238
> -void LayoutTestController::dumpConfigurationForViewport(int availableWidth, int availableHeight)
> +void LayoutTestController::dumpConfigurationForViewport(int dpi, int width, int height, int availableWidth, int availableHeight)

dpi -> deviceDPI

[Kenneth Rohde Christiansen]

Created attachment 83650 [details]
Patch 2

[WebKit Review Bot]

Attachment 83650 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:61:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 134 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Collabora GTK+ EWS bot]

Attachment 83650 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8031109

[Kenneth Rohde Christiansen]

GTK uses the shared header file so I'm going to update all users of this one.

[Kenneth Rohde Christiansen]

Created attachment 83781 [details]
Patch 3

[WebKit Review Bot]

Attachment 83781 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:61:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 140 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Attachment 83781 [details] did not build on win:
Build output: http://queues.webkit.org/results/8034128

[Andreas Kling]

Comment on attachment 83781 [details]
Patch 3

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

> Tools/DumpRenderTree/LayoutTestController.cpp:154
> +    double deviceWidth = JSValueToNumber(context, arguments[1], exception);
> +    ASSERT(!*exception);
> +    double deviceWidth = JSValueToNumber(context, arguments[2], exception);

Oops, both variables are called deviceWidth here!

[Kenneth Rohde Christiansen]

Created attachment 83787 [details]
Patch 4

[WebKit Review Bot]

Attachment 83787 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:61:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 140 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 83787 [details]
Patch 4

Clearing flags on attachment: 83787

Committed r79783: <http://trac.webkit.org/changeset/79783>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Mihai Parparita]

It looks like this didn't check in any -expected files for the new tests (viewport-91.html, etc.).

[Kenneth Rohde Christiansen]

(In reply to comment #16)
> It looks like this didn't check in any -expected files for the new tests (viewport-91.html, etc.).

Oh, that is a mistake. Let me check them in tomorrow.

[WebKit Review Bot]

http://trac.webkit.org/changeset/79783 might have broken GTK Linux 32-bit Debug