Bug 74014 - Allow deviceScaleFactor to be set from window.internals
Summary: Allow deviceScaleFactor to be set from window.internals
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fady Samuel
URL:
Keywords:
Depends on:
Blocks: 70559
  Show dependency treegraph
 
Reported: 2011-12-07 11:41 PST by Fady Samuel
Modified: 2012-02-21 12:41 PST (History)
7 users (show)

See Also:


Attachments
Patch (5.43 KB, patch)
2011-12-07 12:48 PST, Fady Samuel
no flags Details | Formatted Diff | Diff
Patch (5.42 KB, patch)
2011-12-07 13:04 PST, Fady Samuel
no flags Details | Formatted Diff | Diff
Patch (5.56 KB, patch)
2012-01-05 09:20 PST, Fady Samuel
no flags Details | Formatted Diff | Diff
Patch (7.07 KB, patch)
2012-01-05 09:53 PST, Fady Samuel
morrita: review-
gns: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fady Samuel 2011-12-07 11:41:50 PST
Some deviceScaleFactor related bugs are cropping up, so we need to make sure this is testable.
Comment 1 Fady Samuel 2011-12-07 12:48:01 PST
Created attachment 118254 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-12-07 12:52:56 PST
Comment on attachment 118254 [details]
Patch

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

> Source/WebCore/testing/Internals.idl:88
> +        float getDeviceScaleFactor(in Document document) raises(DOMException);

Drop the 'get'
Comment 3 Fady Samuel 2011-12-07 13:04:06 PST
Created attachment 118259 [details]
Patch
Comment 4 Fady Samuel 2011-12-07 13:05:06 PST
(In reply to comment #2)
> (From update of attachment 118254 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118254&action=review
> 
> > Source/WebCore/testing/Internals.idl:88
> > +        float getDeviceScaleFactor(in Document document) raises(DOMException);
> 
> Drop the 'get'

Done. I uploaded a new patch to make sure bots go green first.
Comment 5 WebKit Review Bot 2011-12-07 13:05:53 PST
Attachment 118259 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 26f0902d4a952ed5da5ec33d1949597fa79abdda != e9bcf9b2e57e236c20b02d45c6e274893b01a1f7
rereading f1b18930bbf5e73c83f2b0018b501e8f64330752
	M	Source/JavaScriptCore/wtf/HashTraits.h
	M	Source/JavaScriptCore/ChangeLog
	M	Source/WebCore/dom/ChildListMutationScope.cpp
	M	Source/WebCore/ChangeLog
102267 = 67d4f2de4c2f4a2b6f1b3adfd20b50620fad3051 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Gustavo Noronha (kov) 2011-12-07 14:34:23 PST
Comment on attachment 118259 [details]
Patch

Attachment 118259 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10750291
Comment 7 Fady Samuel 2012-01-05 09:20:08 PST
Created attachment 121288 [details]
Patch
Comment 8 Fady Samuel 2012-01-05 09:21:52 PST
I have refreshed the patch. Hopefully the EWS bots are happy.
Comment 9 Gustavo Noronha (kov) 2012-01-05 09:26:39 PST
Comment on attachment 121288 [details]
Patch

Attachment 121288 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11120421
Comment 10 Fady Samuel 2012-01-05 09:53:38 PST
Created attachment 121289 [details]
Patch
Comment 11 Fady Samuel 2012-01-05 09:54:18 PST
Using EWS bots to fix build issues on other platforms...
Comment 12 Gustavo Noronha (kov) 2012-01-05 10:00:54 PST
Comment on attachment 121289 [details]
Patch

Attachment 121289 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11144011
Comment 13 Hajime Morrita 2012-01-16 00:45:37 PST
Comment on attachment 121289 [details]
Patch

Please use LayoutTestController instead of Internals because setting/getting the device scale factor can be done by WebKit public API.
The Internals object is for inspecting hidden, internal state of WebCore.

It's OK to take care of Chromium for now, skip for other ports if you file bugs for them.
Comment 14 Fady Samuel 2012-02-17 15:47:04 PST
(In reply to comment #13)
> (From update of attachment 121289 [details])
> Please use LayoutTestController instead of Internals because setting/getting the device scale factor can be done by WebKit public API.
> The Internals object is for inspecting hidden, internal state of WebCore.
> 
> It's OK to take care of Chromium for now, skip for other ports if you file bugs for them.

windows.internals is cross platform. There's already page scaling code in windows.internals, and this code is related.

There's no reason why this parameter has to behave differently between platforms.
Comment 15 Kenneth Rohde Christiansen 2012-02-18 06:24:43 PST
I guess not all platforms will make a public API for setting this but instead hardcode it per platform, so I guess internals is totally ok for this.
Comment 16 Fady Samuel 2012-02-21 12:41:02 PST
(In reply to comment #13)
> (From update of attachment 121289 [details])
> Please use LayoutTestController instead of Internals because setting/getting the device scale factor can be done by WebKit public API.
> The Internals object is for inspecting hidden, internal state of WebCore.
> 
> It's OK to take care of Chromium for now, skip for other ports if you file bugs for them.

Morita, 

This is a fairly cross-platform setting and introducing a layout test suite for scaling and fixed layout seems to be beneficial for many ports of WebKit. Do you think it makes sense to put this into window.internals? There was a thread on webkit-dev about this a few minutes ago and the consensus seemed to be moving code into window.internals is a good idea overall. Thanks.