Bug 100182 - Fix windowRect calculation for webplugin due to page scale factors
Summary: Fix windowRect calculation for webplugin due to page scale factors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-23 19:38 PDT by Min Qin
Modified: 2012-11-12 18:10 PST (History)
3 users (show)

See Also:

Attachments
Patch (3.06 KB, patch)
2012-10-23 19:52 PDT, Min Qin 		no flags Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2012-11-12 13:29 PST, Min Qin 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Min Qin 2012-10-23 19:38:55 PDT 
Fix windowRect calculation for webplugin due to page scale factors
Comment 1 Min Qin 2012-10-23 19:52:14 PDT 
Created attachment 170296 [details]
Patch
Comment 2 Adam Barth 2012-10-23 23:44:36 PDT 
Comment on attachment 170296 [details]
Patch

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

> Source/WebKit/chromium/ChangeLog:12
> +        No test for this as the code for page scale factor will soon be deprecated.

Can we add tests that will continue to pass even after we deprecate page scale factor?  If this code is worth changing, it is worth writing tests for.
Comment 3 Min Qin 2012-10-24 18:33:23 PDT 
I am looking into adding a layout test for this, but the TestWebPlugin failed miserably on this case. 
Whenever I put a x-webkit-test-webplugin in an iframe, i got a context lost error when running the layout test. I will take a look into this
Comment 4 Alexandre Elias 2012-11-12 12:46:16 PST 
Hi Adam, this is difficult to write a test for because it's pixel-based, and plugin placeholder is managed on the Chromium side (so we can't make it render in a layout test).  Could we land without test?
Comment 5 Adam Barth 2012-11-12 13:10:38 PST 
Comment on attachment 170296 [details]
Patch

Ok.  Can you change the explanation in the ChangeLog entry?  That's a much better reason than the code being deprecated soon.  :)
Comment 6 Min Qin 2012-11-12 13:29:30 PST 
Created attachment 173706 [details]
Patch
Comment 7 Min Qin 2012-11-12 13:30:06 PST 
Done, ChangeLog updated.

(In reply to comment #5)
> (From update of attachment 170296 [details])
> Ok.  Can you change the explanation in the ChangeLog entry?  That's a much better reason than the code being deprecated soon.  :)
Comment 8 Adam Barth 2012-11-12 13:37:58 PST 
Comment on attachment 173706 [details]
Patch

Thanks.
Comment 9 WebKit Review Bot 2012-11-12 15:54:12 PST 
Comment on attachment 173706 [details]
Patch

Rejecting attachment 173706 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/14809620
Comment 10 WebKit Review Bot 2012-11-12 18:10:09 PST 
Comment on attachment 173706 [details]
Patch

Clearing flags on attachment: 173706

Committed r134334: <http://trac.webkit.org/changeset/134334>
Comment 11 WebKit Review Bot 2012-11-12 18:10:13 PST 
All reviewed patches have been landed.  Closing bug.