837399 - Port bug 774315 (ability to hide placeholder for click to play)

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[[e-mail]]

* Close box doesn't work
* No Modern theme support for the close box
* No context menu options to play/hide plugin

Comment 1

[[e-mail]]

Attachment: WIP

If you can't wait for the context menu changes, feel free to r+ this (if appropriate) and I'll attach the context menu changes later.

Comment 2

[[e-mail]]

Attachment: Right-click on CTP plugin activates it

Necessary before we can even think about giving it a context menu entry, and we probably also want to land this part on branches too.

Comment 3

[Philip Chee]

Comment on attachment 709383 [details] [diff] [review]
WIP

close button is invisible in modern.

> -            for (let plugin of plugins) {
> -              let overlay = doc.getAnonymousElementByAttribute(plugin, "class", "mainBox");
> +            for (var plugin of plugins)
Any reason for changing this from let to var?

Comment 4

[[e-mail]]

Attachment: Right-click on CTP plugin activates it

Now checking the correct button (oops!)

Comment 5

[[e-mail]]

Attachment: Make the close box work

In Classic, at least... I'll patch Modern separately.

Comment 6

[[e-mail]]

(In reply to Philip Chee from comment #3)
> (From update of attachment 709383 [details] [diff] [review])
> > +            for (var plugin of plugins)
> Any reason for changing this from let to var?
I had to edit the line anyway ;-)

Comment 7

[[e-mail]]

Attachment: Modern theme support

Including the necessary bits of bug 831921 (i.e. excluding the unnecessary bits, which looked awful when I tried it out...)

Comment 8

[Philip Chee]

Comment on attachment 711520 [details] [diff] [review]
Make the close box work

r=me

Comment 9

[Philip Chee]

Comment on attachment 711553 [details] [diff] [review]
Modern theme support

>    color: white;
[I think modern should use #FFFFFF].

> +  text-shadow: 0 0 3.5px rgba(0, 0, 0, 0.8);
>    border-radius: 12px;
You have a radius of 12px but our close.gif is pretty square-ish so it looks too close to the rounded corner.

In winstripe further down I see:


> :-moz-handler-clicktoplay .mainBox {
....
>   border-radius: 2.5px;
Setting the border radius to 2.5px makes our close.gif look more snug.

....
>   text-shadow: none;
>   box-shadow: none;
I am unsure if the above two styles make the CTP box look better or worse.

>  :-moz-handler-clicktoplay .icon,
So I assume you decided not to set this to contentPluginClickToPlayPlain.png like winstripe. Any reason? OK it looks pretty ugly to me.

>  :-moz-handler-vulnerable-updatable .icon,
>  :-moz-handler-vulnerable-no-update .icon {
>    background-image: url(chrome://mozapps/skin/plugins/contentPluginClickToPlay.png);

Comment 10

[Philip Chee]

Comment on attachment 711553 [details] [diff] [review]
Modern theme support

>> :-moz-handler-clicktoplay .mainBox {
> ....
>>   border-radius: 2.5px;
> Setting the border radius to 2.5px makes our close.gif look more snug.
r=me with the border radius set to 6px as discussed over IRC.

Comment 11

[[e-mail]]

Comment on attachment 711520 [details] [diff] [review]
Make the close box work

Pushed comm-central changeset 918a7b8058ee.

Comment 12

[[e-mail]]

Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset 8a31b4fc6ce4.

Comment 13

[[e-mail]]

Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset d2957730cfd1 for the review comment.

Comment 14

[[e-mail]]

I'm going to resolve this and open a new bug for the context menu.

Comment 15

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] from comment #14)
> I'm going to (...) open a new bug for the context menu.

Did you?

Comment 16

[[e-mail]]

(In reply to Jens Hatlak from comment #15)
> (In reply to comment #14)
> > I'm going to (...) open a new bug for the context menu.
> 
> Did you?

No, sorry. I've just filed bug 858995.