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)
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)
41.95 KB,
image/png
|
Details | |
3.41 KB,
patch
|
Gavin
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•12 years ago
|
Attachment #633336 -
Flags: review?(mbrubeck)
Updated•12 years ago
|
Attachment #633336 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 1•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/17e72edbf994
Comment 2•12 years ago
|
||
It looks like this regression is on Aurora too.
Assignee: nobody → wjohnston
Blocks: 760696
Status: NEW → ASSIGNED
status-firefox15:
--- → affected
Keywords: regression
Version: Trunk → Firefox 15
Assignee | ||
Comment 3•12 years ago
|
||
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?
Comment 4•12 years ago
|
||
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
Assignee | ||
Comment 5•12 years ago
|
||
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?
Assignee | ||
Comment 6•12 years ago
|
||
Screenshot. Also will push this to try. Also, forgot to port the styles change to XUL, so another patch is coming.
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #633595 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #633600 -
Flags: review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/57fd3ee041cf
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/57fd3ee041cf
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Assignee | ||
Comment 10•12 years ago
|
||
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?
Comment 11•12 years ago
|
||
(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?
Assignee | ||
Comment 12•12 years ago
|
||
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?
Comment 13•12 years ago
|
||
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
Comment 14•12 years ago
|
||
This is just making use of strings that have already been added - this patch alone has no l10n impact.
Comment 15•12 years ago
|
||
(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.
Updated•12 years ago
|
Attachment #633600 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•12 years ago
|
||
Did it land in Aurora?
Assignee | ||
Comment 17•12 years ago
|
||
No.... Did now though: http://hg.mozilla.org/releases/mozilla-aurora/rev/f5a50e834842
Updated•12 years ago
|
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•