Last Comment Bug 697124 - Update the Context menu video items.
: Update the Context menu video items.
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.7
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks: 712871
  Show dependency treegraph
 
Reported: 2011-10-25 08:31 PDT by Philip Chee
Modified: 2011-12-22 20:06 PST (History)
0 users
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Patch v1.0 (14.42 KB, text/plain)
2011-10-25 09:02 PDT, Philip Chee
neil: review+
Details
Patch v1.1 nits fixed. r=Neil (39.82 KB, patch)
2011-10-27 02:39 PDT, Philip Chee
philip.chee: review+
Details | Diff | Review

Description Philip Chee 2011-10-25 08:31:10 PDT
q.v. Firefox bugs:
Bug 681550 - Add ability to save current frame of video.
  (Typo fix) Bug 693099 - "Save Snapshot As" option on HTML5 videos creates PNG files with .JPG file extension.

Bug 481082 -  Video controls listen for stalled event and change the UI accordingly.

Bug 669260 - Add statistics overlay to video element.
  Bug 692640 - Video statistics text overlaps itself on small dimension videos.
Comment 1 Philip Chee 2011-10-25 09:02:46 PDT
Created attachment 569398 [details]
Patch v1.0

> +  saveVideoFrameAsImage: function () {
> +    urlSecurityCheck(this.mediaURL, this.browser.contentPrincipal,

Couldn't get past urlSecurityCheck when using |this.target.nodePrincipal|

> +                     Components.interfaces.nsIScriptSecurityManager.DISALLOW_SCRIPT);

Firefox uses .DISALLOW_SCRIPT. I notice that we use .ALLOW_CHROME in this file instead.

>      return form.method == "get" || (form.method == "post" &&
>             form.enctype == "application/x-www-form-urlencoded");
>    },
> -  
> +

Only one instance of trailing whitespace, not worth another bug just to fix this.

> +html|table {
> +  font-family: Helvetica, Ariel, sans-serif;
> +  font-size: 11px;
> +  color: #FFFFFF;
> +  text-shadow:
> +    -1px -1px 0 #000000,
> +    1px -1px 0 #000000,
> +    -1px 1px 0 #000000,
> +    1px 1px 0 #000000;
> +  min-width: 100%;
> +  background: rgba(68,68,68,.7);
> +  table-layout: fixed;
> +  border-collapse: collapse;
> +  position: absolute;
> +}

Copied from winstripe. No idea what colours are appropriate for Modern.

>    skin/modern/global/media/throbber.png                            (/mozilla/toolkit/themes/winstripe/global/media/throbber.png)
> +  skin/modern/global/media/stalled.png                             (/mozilla/toolkit/themes/winstripe/global/media/stalled.png)

I see we directly copy some PNGs from winstripe so I'm doing the same here. stalled.png is already optimized AFAICT.
Comment 2 neil@parkwaycc.co.uk 2011-10-26 16:28:30 PDT
Comment on attachment 569398 [details]
Patch v1.0

>+    var name = "";
var name = "snapshot.jpg";
Avoids having to test it later.

>+/* Statistics formatting */
>+html|*.statsDiv {
>+  position: relative;
>+}
>+html|td {
>+  height: 1em;
>+  max-height: 1em;
>+  padding: 0 2px;
>+}
>+html|table {
Nit: line spacing between blocks

>+  font-family: Helvetica, Ariel, sans-serif;
Typo: Arial

>+  text-shadow:
>+    -1px -1px 0 #000000,
>+    1px -1px 0 #000000,
>+    -1px 1px 0 #000000,
>+    1px 1px 0 #000000;
I'd prefer this on one line, even if it is slightly more than 80 characters.

>+  skin/modern/global/media/stalled.png                             (/mozilla/toolkit/themes/winstripe/global/media/stalled.png)
Please copy the file, in case toolkit changes things. r=me with those fixed.
[optipng won't touch the file because it's an APNG]
Comment 3 Philip Chee 2011-10-27 02:39:11 PDT
Created attachment 569920 [details] [diff] [review]
Patch v1.1 nits fixed. r=Neil

>>+    var name = "";
> var name = "snapshot.jpg";
> Avoids having to test it later.
Fixed.

>>+/* Statistics formatting */
>>+html|*.statsDiv {
>>+  position: relative;
>>+}
>>+html|td {
>>+  height: 1em;
>>+  max-height: 1em;
>>+  padding: 0 2px;
>>+}
>>+html|table {
> Nit: line spacing between blocks
Fixed.

>>+  font-family: Helvetica, Ariel, sans-serif;
> Typo: Arial
Fixed.

>>+  text-shadow:
>>+    -1px -1px 0 #000000,
>>+    1px -1px 0 #000000,
>>+    -1px 1px 0 #000000,
>>+    1px 1px 0 #000000;
> I'd prefer this on one line, even if it is slightly more than 80 characters.
Fixed.

>>+  skin/modern/global/media/stalled.png                             (/mozilla/toolkit/themes/winstripe/global/media/stalled.png)
> Please copy the file, in case toolkit changes things. r=me with those fixed.
> [optipng won't touch the file because it's an APNG]
Fixed.
Comment 4 Philip Chee 2011-10-27 02:45:38 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/a83e6190537e
Comment 5 Serge Gautherie (:sgautherie) 2011-12-21 23:11:16 PST
Update of test will happen in bug 712871.
Comment 6 Justin Dolske [:Dolske] 2011-12-22 20:06:52 PST
Please don't spam Firefox/Toolkit bugs with useless dependencies.

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