861112 - Flash plugin incorrectly positioned when using non-standard layout.css.devPixelsPerPx value

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Jonathan Kew [:jfkthame]]

See bug 840881 comment 20, and attachment 735843 [details]. Since bug 840881 was landed to fix menu/popup scaling issues, Flash objects appear to be rendered with an incorrectly-scaled position (although the size is correct).

I'd guess widget/gtk2/nsWindow.cpp probably needs to distinguish plugin windows from normal top-level widgets, and avoid the CSS-to-devpix scaling for their position.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch, scaling from display to device pixels should only apply to top-level windows/dialogs/popups, not to child or plugin windows

This is simply the gtk2 version of bug 821454 on win32, and the same fix of distinguishing top-level windows from other widgets applies here as well. With this patch, youtube flash videos stay in the proper place on the page.

Comment 2

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0e3cd48151b

Comment 3

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a0e3cd48151b

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 736837 [details] [diff] [review]
patch, scaling from display to device pixels should only apply to top-level windows/dialogs/popups, not to child or plugin windows

As pointed out by Carl in bug 861270 comment 19, we've fixed some of the issues for Linux users who have custom (hidpi) devPixelsPerPx settings, but this one is still present up through FF22.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): hidpi support

User impact if declined: incorrect display of Flash and other plugins for Linux users with hidpi configurations

Testing completed (on m-c, etc.): in Nightly for 10 days

Risk to taking this patch (and alternatives if risky): minimal risk - simple, localized patch, also directly equivalent to what we have on Windows (bug 821454), which landed for mozilla20).

String or IDL/UUID changes made by this patch: none

Comment 5

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a3c55bdbe37d

Comment 6

[bhavana bajaj [:bajaj]]

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> Comment on attachment 736837 [details] [diff] [review]
> patch, scaling from display to device pixels should only apply to top-level
> windows/dialogs/popups, not to child or plugin windows
> 
> As pointed out by Carl in bug 861270 comment 19, we've fixed some of the
> issues for Linux users who have custom (hidpi) devPixelsPerPx settings, but
> this one is still present up through FF22.
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): hidpi support
> 
> User impact if declined: incorrect display of Flash and other plugins for
> Linux users with hidpi configurations
> 
> Testing completed (on m-c, etc.): in Nightly for 10 days
> 
> Risk to taking this patch (and alternatives if risky): minimal risk -
> simple, localized patch, also directly equivalent to what we have on Windows
> (bug 821454), which landed for mozilla20).

What are the chances of this code causing any new regressions, would that be limited to linux-HIDPI users alone? 
> 
> String or IDL/UUID changes made by this patch: none

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to bhavana bajaj [:bajaj] from comment #6)

> What are the chances of this code causing any new regressions, would that be
> limited to linux-HIDPI users alone? 

Yes, AFAICT there's no possibility of affecting any other users. The change is in the platform-specific Gtk2 widget code only, so it cannot affect other platforms. What the change does is simply to decide whether to apply the "default scale factor" to widget coordinates. For users with default (non-hidpi) pref settings, this factor is 1.0 and therefore the choice makes no difference to behavior; only those who have adjusted the devPixelsPerPx pref will be affected by the conditionals added here.

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 736837 [details] [diff] [review]
patch, scaling from display to device pixels should only apply to top-level windows/dialogs/popups, not to child or plugin windows

Please land by tomorrow morning PT to get this into our second last beta for Fx21.

Comment 9

[u279076]

Is there a range of factors we should be testing? Does this require Hi-DPI hardware or can it be tested on anything with various pref values?

Comment 10

[Jonathan Kew [:jfkthame]]

You should be able to reproduce the problem by setting layout.css.devPixelsPerPx to a non-default value (e.g. 1.1, 1.2, 1.5, etc) on any Linux/Gtk system, regardless of the actual dpi of the hardware; all that matters is that you force a non-1:1 scaling factor.

Comment 11

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-beta/rev/49c838b4b1d8

Comment 12

[Cornel Ionce [:noni] [Hubs QA]]

Verified as fixed on Firefox 21 beta 6 (Build ID: 20130430204233), using various pref values and a non Hi-DPI display.

User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0

Comment 13

[u279076]

(In reply to Cornel Ionce [QA] from comment #12)
> Verified as fixed on Firefox 21 beta 6 (Build ID: 20130430204233), using
> various pref values and a non Hi-DPI display.
> 
> User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101
> Firefox/21.0

Thanks Cornel. Please verify this on the latest Aurora and Nightly builds when you get a chance.

Comment 14

[Carl Thompson]

Works for me also on Hi-DPI display (Macbook Pro Retina).

Comment 15

[u279076]

(In reply to Carl Thompson from comment #14)
> Works for me also on Hi-DPI display (Macbook Pro Retina).

Thanks Carl, which Firefox versions did you test?

Comment 16

[Carl Thompson]

Same version as the previous tester: 21b6. Thanks!

Comment 17

[u279076]

Would you be able to also test the latest Aurora and Nightly builds?
http://nightly.mozilla.org
http://aurora.mozilla.org

Comment 18

[Carl Thompson]

The nightlies have worked for a while now (that's what I was using to get these fixes to get the fixes in this beta). I'll try to test Aurora this evening.

Comment 19

[Cornel Ionce [:noni] [Hubs QA]]

Also verified on latest Aurora (Build ID: 20130502004015) and on latest Nightly (Build ID: 20130502030939). 

Marking as Verified Fixed.

Comment 20

[Bence Szalai]

I have the same issue on v59.0.2 (64-bit) on Mac OSX. Looks like this is a long standing one.