Closed
Bug 890516
Opened 11 years ago
Closed 11 years ago
Embedded audio stopped working after installing FF 22
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: jonathan, Assigned: mounir)
References
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
223 bytes,
text/html
|
Details | |
6.78 KB,
patch
|
bzbarsky
:
review+
bajaj
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release) Build ID: 20130618035212 Steps to reproduce: Clicked on button marked 'Hear Hebrew' in the URL: http://www.messianictrust.org.uk/parashiyot/mattot-10.php Same problem can be seen by clicking over any of the Hebrew word graphics in the text or clicking the 'Hear Hebrew (HQ, slower)' button Actual results: The sound file did embed, but made no sound Expected results: The sound file should have played
Comment 1•11 years ago
|
||
Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/015da7030aab Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130304 Firefox/22.0 ID:20130304232049 Bad: http://hg.mozilla.org/mozilla-central/rev/c95439870e05 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130305 Firefox/22.0 ID:20130305072450 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=015da7030aab&tochange=c95439870e05 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/c43e96e4a41a Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130304 Firefox/22.0 ID:20130304214548 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/f73c5ece0869 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130304 Firefox/22.0 ID:20130305012649 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c43e96e4a41a&tochange=f73c5ece0869 Regressed by: f73c5ece0869 Mounir Lamouri — Bug 614825 - <embed> should be display:none; when hidden attribute is set. r=bz
Blocks: 614825
Status: UNCONFIRMED → NEW
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
status-firefox25:
--- → affected
Component: Untriaged → DOM: Core & HTML
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Comment 2•11 years ago
|
||
WORKAROUND usrContent.css: embed { display:-moz-box !important; }
Comment 3•11 years ago
|
||
So I just checked, and @hidden on <embed> does NOT make it display:none in WebKit either. Mounir, maybe this is just not web-compatible at all and the spec needs to be changed here...
Flags: needinfo?(mounir)
Assignee | ||
Comment 4•11 years ago
|
||
Oh boy... So, Webkit and Blink do not make the element display:none indeed. They only put CSS width and height to 0. Not when @hidden is set but when @hidden is set to "yes" or "true": https://github.com/WebKit/webkit/blob/master/Source/WebCore/html/HTMLEmbedElement.cpp#L84 https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/HTMLEmbedElement.cpp&l=84 I wasn't able to really understand what IE is doing (because I use browserstack and their devtools do not look great or changing @hidden has no effect). Same kind of problem with Opera/Presto: the embed element never gets added to the DOM. I would be very interested to know what IE is doing. If they have the same behaviour as Webkit and Blink I think we have no choice but to add this to the specs and implement it.
Flags: needinfo?(mounir)
Comment 5•11 years ago
|
||
IE11 displayed "inline".
Comment 6•11 years ago
|
||
The previous patch didn't work on Bugzilla because bugzilla.mozilla.org is in the Compatibility View list.
Attachment #771884 -
Attachment is obsolete: true
Assignee | ||
Comment 7•11 years ago
|
||
So, IE9/10 behaviour is the following: - always display: inline; - if @hidden is set to the empty string or "true", hide the element. Webkit/Blink does: - always display: inline; - if @hidden is set to "yes" or "true", hide the element. I think we could safely try to push something where @hidden is followed if set to the empty string, "yes" or "true". We could probably try to follow @hidden if set to anything but "false" or "no".
Assignee | ||
Comment 8•11 years ago
|
||
This is building so it is not tested yet but I guess it should work. I will open a follow-up wrt implementing the right thing.
Assignee | ||
Updated•11 years ago
|
OS: Windows XP → All
Hardware: x86 → All
Comment 9•11 years ago
|
||
Comment on attachment 772204 [details] [diff] [review] Patch MapCommonAttributesInto is a static function, so how can it use Tag()? You want to go back to the setup we had before bug 614825.
Attachment #772204 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #772204 -
Attachment is obsolete: true
Attachment #773326 -
Flags: review?(bzbarsky)
Comment 11•11 years ago
|
||
Comment on attachment 773326 [details] [diff] [review] Patch >+MapAttributesIntoRuleBase(const nsMappedAttributes *aAttributes, > nsRuleData *aData) Fix indent, please. >+MapAttributesIntoRuleExceptHidden(const nsMappedAttributes *aAttributes, >+ nsRuleData *aData) And here. r=me
Attachment #773326 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6425dcad7f9e
Flags: in-testsuite+
Target Milestone: --- → mozilla25
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 773326 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 614825 User impact if declined: some websites don't work Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low, no alternatives String or IDL/UUID changes made by this patch: no
Attachment #773326 -
Flags: approval-mozilla-beta?
Attachment #773326 -
Flags: approval-mozilla-aurora?
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6425dcad7f9e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 15•11 years ago
|
||
Comment on attachment 773326 [details] [diff] [review] Patch low risk enough and a recent regression in Fx22.Approving on aurora and requesting qa verification
Attachment #773326 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
tracking-firefox23:
--- → +
tracking-firefox24:
--- → +
Comment 16•11 years ago
|
||
Comment on attachment 773326 [details] [diff] [review] Patch Recent regression in FF22, so approving for Beta as well given where we are in the cycle.
Attachment #773326 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•11 years ago
|
||
Thank you. This kind of response to issues the main reason I remain a loyal Firefox advocate, besides the fact that FF is the only browser that fully supports MathML (Safari only partially implements it, and Chrome seems to have given up and has removed support..) Great job all!
Assignee | ||
Comment 19•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f1cf071add8e https://hg.mozilla.org/releases/mozilla-beta/rev/c653726e0cc8
You need to log in
before you can comment on or make changes to this bug.
Description
•