Closed Bug 765106 Opened 12 years ago Closed 12 years ago

Video controls are using the desktop binding

Categories

(Firefox for Android Graveyard :: General, defect)

15 Branch
defect
Not set
normal

Tracking

(firefox15 fixed)

RESOLVED FIXED
Firefox 16
Tracking Status
firefox15 --- fixed

People

(Reporter: wesj, Assigned: wesj)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Attached file Patch (obsolete) —
Desktop recently started using -moz-any(video,audio) to attach their video controls binding. -moz-any is more specific than the video,audio we use, breaking ours.
Attachment #633336 - Flags: review?(mbrubeck)
Attachment #633336 - Flags: review?(mbrubeck) → review+
It looks like this regression is on Aurora too.
Assignee: nobody → wjohnston
Blocks: 760696
Status: NEW → ASSIGNED
Keywords: regression
Version: Trunk → Firefox 15
Comment on attachment 633336 [details]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 659402
User impact if declined: Video controls do not behave correctly (fade in and out) and have the wrong layout on mobile
Testing completed (on m-c, etc.): Landed on mc Jun14
Risk to taking this patch (and alternatives if risky):  Low risk. Gives us back our old beahvior.
String or UUID changes made by this patch:
Attachment #633336 - Flags: approval-mozilla-aurora?
Sorry, I backed this out on inbound because it appeared to cause failures in mochitest-2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c9d11e009b7

https://tbpl.mozilla.org/php/getParsedLog.php?id=12684783&tree=Mozilla-Inbound
16653 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_constants.html | an unexpected uncaught JS exception reported through window.onerror - TypeError: label is null at chrome://global/content/bindings/videocontrols.xml:699
Attached patch Patch v2 (obsolete) — Splinter Review
This adds some strings we ignored before because... we forgot them I guess. I'll carry over mbrubeck's review and get one from someone who can approve toolkit changes.
Attachment #633336 - Attachment is obsolete: true
Attachment #633336 - Flags: approval-mozilla-aurora?
Screenshot. Also will push this to try. Also, forgot to port the styles change to XUL, so another patch is coming.
Attached patch Patch v2.1Splinter Review
Attachment #633595 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/57fd3ee041cf
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Comment on attachment 633600 [details] [diff] [review]
Patch v2.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 659402
User impact if declined: Video controls do not behave correctly (fade in and out) and have the wrong layout on mobile
Testing completed (on m-c, etc.): Landed on mc Jun14
Risk to taking this patch (and alternatives if risky):  Low risk. Gives us back our old beahvior.
String or UUID changes made by this patch:
Attachment #633600 - Flags: approval-mozilla-aurora?
(In reply to Wesley Johnston (:wesj) from comment #10)
> String or UUID changes made by this patch:

Do the changes in videocontrols.xml qualify as strings that will need to be newly localized?
Hmm... These were checked into Firefox 11 (bug 513405). I guess there is a possibility there is a language that we want to ship mobile in that hasn't already translated them for desktop, but I'm guessing no one matches that criteria. Axel, do you know how to check this?
This is copying over the strings from videoControls to touchControls, both used for video elements, but with different UI interactions?

In that case, re-using the strings should be OK.

PS: You can search for strings in l10n on mxr, like http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=error.srcNotSupported
This is just making use of strings that have already been added - this patch alone has no l10n impact.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #14)
> This is just making use of strings that have already been added - this patch
> alone has no l10n impact.

Thanks Gavin.
Attachment #633600 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Did it land in Aurora?
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.