Bug 55142 - Make it possible to test the targetdensity-dpi support
Summary: Make it possible to test the targetdensity-dpi support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Kenneth Rohde Christiansen
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2011-02-24 07:00 PST by Kenneth Rohde Christiansen
Modified: 2011-02-27 09:01 PST (History)
9 users (show)

See Also:


Attachments
Patch (90.95 KB, patch)
2011-02-24 07:02 PST, Kenneth Rohde Christiansen
kling: review+
kling: commit-queue-
Details | Formatted Diff | Diff
Patch 2 (90.99 KB, patch)
2011-02-24 07:17 PST, Kenneth Rohde Christiansen
kling: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
Patch 3 (97.94 KB, patch)
2011-02-25 00:35 PST, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch 4 (97.94 KB, patch)
2011-02-25 02:21 PST, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2011-02-24 07:00:07 PST
SSIA
Comment 1 Kenneth Rohde Christiansen 2011-02-24 07:02:55 PST
Created attachment 83648 [details]
Patch
Comment 2 WebKit Review Bot 2011-02-24 07:04:55 PST
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.
Comment 3 Andreas Kling 2011-02-24 07:09:35 PST
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
Comment 4 Kenneth Rohde Christiansen 2011-02-24 07:17:56 PST
Created attachment 83650 [details]
Patch 2
Comment 5 WebKit Review Bot 2011-02-24 07:19:08 PST
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.
Comment 6 Collabora GTK+ EWS bot 2011-02-25 00:15:05 PST
Attachment 83650 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8031109
Comment 7 Kenneth Rohde Christiansen 2011-02-25 00:31:24 PST
GTK uses the shared header file so I'm going to update all users of this one.
Comment 8 Kenneth Rohde Christiansen 2011-02-25 00:35:47 PST
Created attachment 83781 [details]
Patch 3
Comment 9 WebKit Review Bot 2011-02-25 00:37:26 PST
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.
Comment 10 Build Bot 2011-02-25 01:48:33 PST
Attachment 83781 [details] did not build on win:
Build output: http://queues.webkit.org/results/8034128
Comment 11 Andreas Kling 2011-02-25 02:18:55 PST
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!
Comment 12 Kenneth Rohde Christiansen 2011-02-25 02:21:19 PST
Created attachment 83787 [details]
Patch 4
Comment 13 WebKit Review Bot 2011-02-25 02:22:54 PST
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.
Comment 14 WebKit Commit Bot 2011-02-26 06:39:34 PST
Comment on attachment 83787 [details]
Patch 4

Clearing flags on attachment: 83787

Committed r79783: <http://trac.webkit.org/changeset/79783>
Comment 15 WebKit Commit Bot 2011-02-26 06:39:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Mihai Parparita 2011-02-26 09:12:12 PST
It looks like this didn't check in any -expected files for the new tests (viewport-91.html, etc.).
Comment 17 Kenneth Rohde Christiansen 2011-02-27 04:47:38 PST
(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.
Comment 18 WebKit Review Bot 2011-02-27 09:01:37 PST
http://trac.webkit.org/changeset/79783 might have broken GTK Linux 32-bit Debug