Elbart Reporter
	Description • 11 years ago

STR:
- Set Flash to "ask to activate" (or to "disabled" or not installed at all with a build with bug 921730 fixed)
- Visit URL
- Close the plugin-placeholder

Expected Results:
The placeholder is hidden or not hiding anything else, and the mouseover-menu located above the embed is accessible and usable.

Actual Results:
The gray area of the placeholder remains and covers the mouseover-menu, see screenshot.

	Elbart Reporter
	Comment 1 • 11 years ago

Regression window (m-c):
Last good revision: b1e5ade62913 (2013-12-17)
First bad revision: 862cb6a1cc88 (2013-12-18)
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b1e5ade62913&tochange=862cb6a1cc88

Regression window (m-i):
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4f541dfa1a41&tochange=0be09b045302

Suspected:
97a3f38d2959	Benjamin Smedberg — Bug 853973 - If a plugin is overlayed by other content on the page, then treat it as a hidden plugin: hide the overlay and present the infobar for click-to-play or crashed plugins, r=jaws

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 2 • 11 years ago

Thank you for the details.

The change here is causing this:
http://hg.mozilla.org/integration/mozilla-inbound/diff/97a3f38d2959/toolkit/mozapps/plugins/content/pluginProblemContent.css
Now the overlay itself stays visible and keeps a high z-index and only it's content is hidden.

I'm not sure what the best option is here, but we could e.g.:
* only use the maximum z-index for .mainBox.visible
* actually hide the element or remove the overlay for clicks on .closeIcon or the context menu entry

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 3 • 11 years ago

bsmedberg, what do you think? This still keeps the grey overlay area around but only fixes the z-index when it's "visible".

https://tbpl.mozilla.org/?tree=Try&rev=b5d2f8df2e07

	Benjamin Smedberg
	Comment 5 • 11 years ago

Comment on attachment 8382113 [details] [diff] [review]
Only give "visible" overlay a non-default z-index

+  is(style.zIndex, "2.14748e+9", "Test 1, Plugin overlay should have the maximum z-index.");

This looks kinda fragile. Can we please instead check that it's not 0 or auto?

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 6 • 11 years ago

So, this was failing the test for bug 752516.
The problem is that we now tested for the overlay visibility first (gPluginHandler.shouldShowOverlay) and only then layered it on top using the z-index from .visible.

The next best thing i can think of is:
* defaulting to layering the overlay on top
* still layering on top when we make it "visible"
* fall back to default z-index when we make the overlay "invisible"

https://tbpl.mozilla.org/?tree=Try&rev=e512bf0e60b8

	Benjamin Smedberg
	Comment 7 • 11 years ago

Or, now that we have proper detection for whether a plugin is "on top" we could give up on bug 752516 completely and never use z-index to bring it to the top.

	Anup Kumar [:reznord]
	Comment 8 • 11 years ago

Si, can I have this bug for me because it is like the continuation of the last bug which I have solved. bug 921730

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 9 • 11 years ago

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Or, now that we have proper detection for whether a plugin is "on top" we
> could give up on bug 752516 completely and never use z-index to bring it to
> the top.

That sounds good. With the notification bar & the detection, discoverability is not an issue anymore, so this shouldn't be a problem.

(In reply to Anup from comment #8)
> Si, can I have this bug for me because it is like the continuation of the
> last bug which I have solved. bug 921730

This is more a follow-up to bug 853973 and i already have a patch around. I'm sure we can find further suitable bugs for you after bug 863773!

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 10 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=34d95d9cebfa

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 11 • 11 years ago

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

	Elbart Reporter
	Comment 12 • 11 years ago

Thanks.

After doing some test with the inbound-build (1393931937) and trying some sites, I found an oddity:

bug 927392 also covers the issue of mouseover-menus being covered by the ctp-overlay and has some examples.
Now with the patch of this bug applied, the menu is being shown above the overlay on http://wiesenmarkt.at/news.php , http://www.eleanorlaing.com/ and on http://www.huffingtonpost.com/2012/01/09/taiwan-garbage-trucks-music_n_1195020.html#comments (put the mouse over the Facebook-like-button just below the second embed), but on http://oharenoise.org/ the menu is still behind the overlay.

Is that an issue of the site's code?

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 13 • 11 years ago

(In reply to Elbart from comment #12)
> Is that an issue of the site's code?

Thanks for testing this - it's another issue with the styling of our overlay.
I'm filing a follow-up bug in a bit.

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 11 years ago

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

	Paul Silaghi, QA [:pauly]
	Comment 15 • 11 years ago

From what I see, this bug is not actually about "closing the placeholder", right ?
About "closing the placeholder", I find a little strange the part when clicking on the overlay after closing it from the X button -> the CTP popup and the "Activate" text show up. Is this covered by bug 979318 ?

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 16 • 11 years ago

The behavior for the close icon changed with bug 853973: it now only hides the contents of the overlay, not the overlay itself.
This bug was fixed by not having the overlay stay on top after you clicked the close icon.

The bug you found is something else and should be filed. It should be fixed by what i want to do in bug 979318, so it depends on that bug.

	Paul Silaghi, QA [:pauly]
	Comment 17 • 11 years ago

(In reply to Georg Fritzsche [:gfritzsche] from comment #16)
> This bug was fixed by not having the overlay stay on top after you clicked
> the close icon.
It's not staying on top without clicking the close button too.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 18 • 11 years ago

Yes, indeed, that's how it is supposed to work.

	Paul Silaghi, QA [:pauly]
	Comment 19 • 11 years ago

(In reply to Elbart from comment #0)
> Expected Results:
> the mouseover-menu located above the embed is accessible and usable.
Verified fixed 30.0a1 (2014-03-07), win 7, ubuntu 13.04 and mac os x 10.9

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 20 • 11 years ago

Comment on attachment 8383122 [details] [diff] [review]
Stop layering the plugin overlay topmost

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

User impact if declined: Unreachable elements like navigation menus on some sites due to the plugin placeholder (click-to-play overlay) being on top and no way to access them without activating plugins.

Testing completed (on m-c, etc.): Baked on m-c, verified.

Risk to taking this patch (and alternatives if risky): Low - small style change & we fall back to the "hidden plugin" handling in affected cases where overlays might not be reachable anymore.

String or IDL/UUID changes made by this patch: None.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 21 • 11 years ago

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