Closed Bug 861112 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Widget: Gtk, defect)

x86
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox21 --- verified
firefox22 --- verified

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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.
Assignee: nobody → jfkthame
Attachment #736837 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/a0e3cd48151b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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
Attachment #736837 - Flags: approval-mozilla-beta?
Attachment #736837 - Flags: approval-mozilla-aurora?
Attachment #736837 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
(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
(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 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.
Attachment #736837 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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?
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.
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
(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.
Works for me also on Hi-DPI display (Macbook Pro Retina).
(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?
Same version as the previous tester: 21b6. Thanks!
Would you be able to also test the latest Aurora and Nightly builds?
http://nightly.mozilla.org
http://aurora.mozilla.org
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.
Also verified on latest Aurora (Build ID: 20130502004015) and on latest Nightly (Build ID: 20130502030939). 

Marking as Verified Fixed.
Status: RESOLVED → VERIFIED
Keywords: verifyme
I have the same issue on v59.0.2 (64-bit) on Mac OSX. Looks like this is a long standing one.
You need to log in before you can comment on or make changes to this bug.