Closed Bug 837399 Opened 7 years ago Closed 7 years ago

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

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

* Close box doesn't work
* No Modern theme support for the close box
* No context menu options to play/hide plugin
Attached patch WIP (obsolete) — Splinter Review
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.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #709383 - Flags: feedback?(philip.chee)
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.
Attachment #709430 - Flags: review?(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?
Attachment #709383 - Flags: feedback?(philip.chee)
Now checking the correct button (oops!)
Attachment #709430 - Attachment is obsolete: true
Attachment #709430 - Flags: review?(philip.chee)
In Classic, at least... I'll patch Modern separately.
Attachment #709383 - Attachment is obsolete: true
Attachment #711511 - Attachment is obsolete: true
Attachment #711520 - Flags: review?(philip.chee)
(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 ;-)
Including the necessary bits of bug 831921 (i.e. excluding the unnecessary bits, which looked awful when I tried it out...)
Attachment #711553 - Flags: review?(philip.chee)
Comment on attachment 711520 [details] [diff] [review]
Make the close box work

r=me
Attachment #711520 - Flags: review?(philip.chee) → review+
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);
Attachment #711553 - Flags: review?(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.
Attachment #711553 - Flags: review+
Comment on attachment 711520 [details] [diff] [review]
Make the close box work

Pushed comm-central changeset 918a7b8058ee.
Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset 8a31b4fc6ce4.
Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset d2957730cfd1 for the review comment.
I'm going to resolve this and open a new bug for the context menu.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to neil@parkwaycc.co.uk from comment #14)
> I'm going to (...) open a new bug for the context menu.

Did you?
Blocks: 858995
(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.
You need to log in before you can comment on or make changes to this bug.