Last Comment Bug 837399 - Port bug 774315 (ability to hide placeholder for click to play)
: Port bug 774315 (ability to hide placeholder for click to play)
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on:
Blocks: 858995
  Show dependency treegraph
 
Reported: 2013-02-02 07:46 PST by neil@parkwaycc.co.uk
Modified: 2013-04-06 16:18 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP (3.69 KB, patch)
2013-02-02 07:59 PST, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Right-click on CTP plugin activates it (1.15 KB, patch)
2013-02-02 16:36 PST, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Right-click on CTP plugin activates it (1.15 KB, patch)
2013-02-07 14:12 PST, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Make the close box work (3.71 KB, patch)
2013-02-07 14:21 PST, neil@parkwaycc.co.uk
philip.chee: review+
Details | Diff | Splinter Review
Modern theme support (3.96 KB, patch)
2013-02-07 14:57 PST, neil@parkwaycc.co.uk
philip.chee: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2013-02-02 07:46:42 PST
* Close box doesn't work
* No Modern theme support for the close box
* No context menu options to play/hide plugin
Comment 1 neil@parkwaycc.co.uk 2013-02-02 07:59:44 PST
Created attachment 709383 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2013-02-02 16:36:10 PST
Created attachment 709430 [details] [diff] [review]
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 2013-02-07 06:54:51 PST
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 neil@parkwaycc.co.uk 2013-02-07 14:12:12 PST
Created attachment 711511 [details] [diff] [review]
Right-click on CTP plugin activates it

Now checking the correct button (oops!)
Comment 5 neil@parkwaycc.co.uk 2013-02-07 14:21:51 PST
Created attachment 711520 [details] [diff] [review]
Make the close box work

In Classic, at least... I'll patch Modern separately.
Comment 6 neil@parkwaycc.co.uk 2013-02-07 14:55:56 PST
(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 neil@parkwaycc.co.uk 2013-02-07 14:57:18 PST
Created attachment 711553 [details] [diff] [review]
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 2013-02-12 06:57:55 PST
Comment on attachment 711520 [details] [diff] [review]
Make the close box work

r=me
Comment 9 Philip Chee 2013-02-12 07:00:46 PST
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 2013-02-12 07:46:44 PST
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 neil@parkwaycc.co.uk 2013-02-12 11:24:44 PST
Comment on attachment 711520 [details] [diff] [review]
Make the close box work

Pushed comm-central changeset 918a7b8058ee.
Comment 12 neil@parkwaycc.co.uk 2013-02-12 11:25:13 PST
Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset 8a31b4fc6ce4.
Comment 13 neil@parkwaycc.co.uk 2013-02-12 11:30:39 PST
Comment on attachment 711553 [details] [diff] [review]
Modern theme support

Pushed comm-central changeset d2957730cfd1 for the review comment.
Comment 14 neil@parkwaycc.co.uk 2013-02-19 03:02:48 PST
I'm going to resolve this and open a new bug for the context menu.
Comment 15 Jens Hatlak (:InvisibleSmiley) 2013-04-06 08:37:20 PDT
(In reply to neil@parkwaycc.co.uk from comment #14)
> I'm going to (...) open a new bug for the context menu.

Did you?
Comment 16 neil@parkwaycc.co.uk 2013-04-06 16:18:09 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.