88105 2012-06-01 09:48:19 -0700 "display" property value of computed style for audio element without controls is "inline" (not "none") 2022-10-14 05:23:27 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=245611 InRadar P2 Normal --- 1 imasaki silviapf adele ahmad.saleem792 ap bfulgham dglazkov eric.carlson feature-media-reviews imasaki jer.noble rniwa silviapf simon.fraser webkit-bug-importer webkit.review.bot zalan oldest_to_newest 639458 0 imasaki 2012-06-01 09:48:19 -0700 Copied from http://code.google.com/p/chromium/issues/detail?id=130717 What steps will reproduce the problem? 1. Open attached AudioStyleIssue.html in Web browser. 2. Click "Show information" button. 3. You will see 2 alert boxes with information. First contains information about <audio> element with controls, second contains info about <audio> element without controls. What is the expected result? [first alert] Audio element: id = audioWithControls has controls = true display (CSS value) = inline visibility (CSS value) = visible [second alert] Audio element: id = audioWithoutControls has controls = false display (CSS value) = none visibility (CSS value) = visible What happens instead? [first alert] Audio element: id = audioWithControls has controls = true display (CSS value) = inline visibility (CSS value) = visible [second alert] Audio element: id = audioWithoutControls has controls = false display (CSS value) = inline visibility (CSS value) = visible IE and FF: display property of computed style for audio element without controls is "none". Chrome: display property of computed style for audio element without controls is "inline". So, this behaviour is different in major browsers. 639464 1 imasaki 2012-06-01 09:50:59 -0700 Test on different browsers/platforms. Mac 10.7, gc: 19.0.1084.52 and 21.0.1158.0 canary - able to reproduce the issue Mac 10.7, safari - able to reproduce the issue Win7, 19.0.1084.53 - able to reproduce the issue Win7, FF - unable to reproduce the issue 708545 2 161455 silviapfeiffer1 2012-08-30 06:05:14 -0700 Created attachment 161455 Patch 708793 3 161455 eric.carlson 2012-08-30 10:25:14 -0700 Comment on attachment 161455 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161455&action=review > Source/WebCore/html/HTMLMediaElement.cpp:4118 > + if (!isVideo()) { > + if (controls()) > + removeInlineStyleProperty(CSSPropertyDisplay); > + else > + setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); > + } This will change user-applied 'display' style. I don't know if there is any other way to do this, but is it OK? 709164 4 silviapf 2012-08-30 16:50:44 -0700 (In reply to comment #3) > (From update of attachment 161455 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161455&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:4118 > > + if (!isVideo()) { > > + if (controls()) > > + removeInlineStyleProperty(CSSPropertyDisplay); > > + else > > + setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); > > + } > > This will change user-applied 'display' style. I don't know if there is any other way to do this, but is it OK? Can we add someone else to review this who might know a better way? 709839 5 161455 dglazkov 2012-08-31 11:32:13 -0700 Comment on attachment 161455 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161455&action=review >>> Source/WebCore/html/HTMLMediaElement.cpp:4118 >>> + } >> >> This will change user-applied 'display' style. I don't know if there is any other way to do this, but is it OK? > > Can we add someone else to review this who might know a better way? We already do this in HTMLMediaElement::rendererIsNeeded. What's not happening is triggering the re-evaluation of rendererIsNeeded condition when controls() value changes. This in turn means that we need to re-attach the HTMLAudioElement whenever we call configureMediaControls. 716873 6 silviapf 2012-09-10 23:15:30 -0700 (In reply to comment #5) I am confused by what you are suggesting. Could you help me understand? > We already do this in HTMLMediaElement::rendererIsNeeded. I can't seem to understand what you mean by "this". Are you saying that audio elements without @controls already test whether a renderer is required, and have display:none without a @controls attribute? My tests don't confirm this. All audio elements on a page, with or without @controls, current are display:inline. > What's not happening is triggering the re-evaluation of rendererIsNeeded condition when controls() value changes. My tests show that we have the wrong display value even without JavaScript. A simple <audio></audio> has the wrong display value. When we create just an object like this: a = new Audio(); then this object has a display="" no matter whether it has or has no @controls attribute. > This in turn means that we need to re-attach the HTMLAudioElement whenever we call configureMediaControls. Are you suggesting we destroy the shadow subtree for audio elements that have no @controls attribute and with that set to display:"" ? Then recreate the shadow subtree when the HTMLAudioElement get a @controls attribute again? 717243 7 dglazkov 2012-09-11 09:10:57 -0700 (In reply to comment #6) > (In reply to comment #5) > > I am confused by what you are suggesting. Could you help me understand? What I did here is suggest a solution to a behavior bug without looking at the actual problem that you're trying to solve :) So first, I'll demonstrate the behavior bug, and then give you the answer to the actual problem at hand :P Here's a simple test case: http://jsfiddle.net/xdGtZ/ The key thing to watch for is the red background color of the <div> that encloses the <audio>. When we start, there's no 'controls' attribute on the <audio>. As you can see, there's no red in the results pane, telling us that the height of the <div> that encloses the <audio> is 0. This happens because the <audio> itself did not generate a RenderObject (which would in turn inform the intrinsic height of <div>. The HTMLMediaElement::rendererIsNeeded check made sure of this. Now, click the "Add Controls" button. Immediately, the <audio> inflates in height (and width), and you see the red. The RenderObject for <audio> now exists. Now, click the "Remove Controls" button. Technically, we should now go back to no RenderObject. However, we can still clearly see the Red, which means the <audio> is still being rendered. That's a bug. Anytime the "controls" attribute is added or removed, we must ensure that the styles are recalculated and correct decisions about rendering the box or not are made. Further, this still does not solve the reported problem. Since we're not actually changing the styles (we are manipulating directly on RenderObjects), the computed display value stays the same. Luckily, we just happened to have the right technology in our back pocket. It's called CSS -- specifically the UA stylesheet and the :not pseudoclass. Just add these lines to mediaControls.css: audio:not([controls]) { display: none; } And voila! Everything works just as prescribed. As a bonus, you can now remove all that cruft in HTMLMediaElement::rendererIsNeeded. It's no longer necessary. Hurray! 717250 8 eric.carlson 2012-09-11 09:17:08 -0700 (In reply to comment #7) > Luckily, we just happened to have the right technology in our back pocket. It's called CSS -- specifically the UA stylesheet and the :not pseudoclass. Just add these lines to mediaControls.css: > > audio:not([controls]) { > display: none; > } > > And voila! Everything works just as prescribed. As a bonus, you can now remove all that cruft in HTMLMediaElement::rendererIsNeeded. It's no longer necessary. Hurray! Bravo! 717501 9 silviapf 2012-09-11 13:26:55 -0700 Oh, such an elegant solution - why didn't I see it! Thanks. :-) 1887088 10 ahmad.saleem792 2022-07-27 09:57:22 -0700 Updated results from test case present within Chrome bug and changed to JSfiddle: Link - https://jsfiddle.net/c8btuq0L/show ___ *** Safari 15.6 on macOS 12.5: [First Alert] Audio element: id = audioWithControls has controls = true display (CSS value) = inline visibility (CSS value) = visible [Second Alert] Audio element: id = audioWithoutControls has controls = false display (CSS value) = inline <<<<<<<<<<----------------- BUG visibility (CSS value) = visible *** Firefox Nightly 105: [First Alert] Audio element: id = audioWithControls has controls = true display (CSS value) = inline visibility (CSS value) = visible [Second Alert] Audio element: id = audioWithoutControls has controls = false display (CSS value) = none visibility (CSS value) = visible *** Chrome Canary 106: [First Alert] Audio element: id = audioWithControls has controls = true display (CSS value) = inline visibility (CSS value) = visible [Second Alert] Audio element: id = audioWithoutControls has controls = false display (CSS value) = none visibility (CSS value) = visible __________ Plus the bug also exist in the test case from Comment 07 - where adding control make 'control' with "Red background" but removing will keep "Red" background. ________ I just wanted to share updated results to confirm that this bug still exist in Safari 15.6 on macOS 12.5. Thanks! 1887216 11 webkit-bug-importer 2022-07-27 17:58:40 -0700 <rdar://problem/97694222> 1905587 12 ews-feeder 2022-10-14 05:23:21 -0700 Committed 255528@main (13c796fb7f1d): <https://commits.webkit.org/255528@main> Reviewed commits have been landed. Closing PR #5304 and removing active labels. 161455 2012-08-30 06:05:14 -0700 2012-08-31 11:32:13 -0700 Patch bug-88105-20120830230502.patch text/plain 5133 silviapfeiffer1 U3VidmVyc2lvbiBSZXZpc2lvbjogMTI3MTAzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYmMyMzdmM2YwYWNkNjMy ZWJkMDBkNGMxODg0NTE4NThiOGE2MzY5NS4uNzFhY2JjNmYzZTMzMDg0YzA4ZTM2NjQzMzIzZjI4 YTI1ZDk3ZjMxOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA4LTMwICBTaWx2 aWEgUGZlaWZmZXIgIDxzaWx2aWFwZkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgImRpc3BsYXki IHByb3BlcnR5IHZhbHVlIG9mIGNvbXB1dGVkIHN0eWxlIGZvciBhdWRpbyBlbGVtZW50IHdpdGhv dXQgY29udHJvbHMgaXMgImlubGluZSIgKG5vdCAibm9uZSIpCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04ODEwNQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gYW4gYXVkaW8gZWxlbWVudCBoYXMgbm8g Y29udHJvbHMgYXR0cmlidXRlLCBpdCBzaG91bGQgbm90IGJlIHZpc2libGUgb24gdGhlIHBhZ2Uu CisgICAgICAgIEJlZm9yZSB0aGlzIHBhdGNoLCB3aGVuIHdlIHJlbW92ZWQgdGhlICJjb250cm9s cyIgYXR0cmlidXRlLCBhbiBlbXB0eSBkaXYgb2YgdGhlCisgICAgICAgIHNpemUgb2YgdGhlIGNv bnRyb2xzIHJlbWFpbmVkLgorCisgICAgICAgIFRlc3Q6IG1lZGlhL2F1ZGlvLWNvbnRyb2xzLWhp ZGRlbi1kaXNwbGF5LW5vbmUuaHRtbAorCisgICAgICAgICogaHRtbC9IVE1MTWVkaWFFbGVtZW50 LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnQ6OmNvbmZpZ3VyZU1lZGlh Q29udHJvbHMpOgorICAgICAgICBDb25maWd1cmUgYXVkaW8gZWxlbWVudHMgd2l0aG91dCBjb250 cm9scyBhdHRyaWJ1dGUgdG8gaGF2ZSBkaXNwbGF5Om5vbmUuCisKIDIwMTItMDgtMjkgIE5hdCBE dWNhICA8bmR1Y2FAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFtjaHJvbWl1bV0gc2V0TmVlZHNB bmltYXRlIHNob3VsZCBub3QgY2F1c2UgY29tbWl0UmVxdWVzdGVkIHRvIGJlY29tZSB0cnVlCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwIGIvU291 cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAppbmRleCA3Yzg3M2Y1YTA0MzNi MmIxMjExNmJhZjBmYTMwMGZiNjA4Y2EzOWYxLi44Mzk2ZDU2Y2VlMTc4NWQ4ZTUzZWI0ODY4MmY4 ZDU1MGVmNzU1MTI1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVs ZW1lbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAK QEAgLTQxMDksNiArNDEwOSwxNCBAQCBib29sIEhUTUxNZWRpYUVsZW1lbnQ6OmNyZWF0ZU1lZGlh Q29udHJvbHMoKQogCiB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6OmNvbmZpZ3VyZU1lZGlhQ29udHJv bHMoKQogeworICAgIC8vIEF1ZGlvIGVsZW1lbnRzIHdpdGhvdXQgY29udHJvbHMgaGF2ZSBhIGRp c3BsYXkgb2YgTk9ORS4KKyAgICBpZiAoIWlzVmlkZW8oKSkgeworICAgICAgICBpZiAoY29udHJv bHMoKSkKKyAgICAgICAgICAgIHJlbW92ZUlubGluZVN0eWxlUHJvcGVydHkoQ1NTUHJvcGVydHlE aXNwbGF5KTsKKyAgICAgICAgZWxzZQorICAgICAgICAgICAgc2V0SW5saW5lU3R5bGVQcm9wZXJ0 eShDU1NQcm9wZXJ0eURpc3BsYXksIENTU1ZhbHVlTm9uZSk7CisgICAgfQorCiAjaWYgIUVOQUJM RShQTFVHSU5fUFJPWFlfRk9SX1ZJREVPKQogICAgIGlmICghY29udHJvbHMoKSB8fCAhaW5Eb2N1 bWVudCgpKSB7CiAgICAgICAgIGlmIChoYXNNZWRpYUNvbnRyb2xzKCkpCmRpZmYgLS1naXQgYS9M YXlvdXRUZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggMWEwZTZj NzU0ZjRkNGE4Y2ZlNGE3MDQ3YjBiNDk5ZDlmYWUyYzQyMy4uNjE4MGFmNTAwNzc1MWI4YzJlZjhi ZjBjM2RhMDBhMmYxNjUwNTQyMyAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisr KyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA4LTMwICBT aWx2aWEgUGZlaWZmZXIgIDxzaWx2aWFwZkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgImRpc3Bs YXkiIHByb3BlcnR5IHZhbHVlIG9mIGNvbXB1dGVkIHN0eWxlIGZvciBhdWRpbyBlbGVtZW50IHdp dGhvdXQgY29udHJvbHMgaXMgImlubGluZSIgKG5vdCAibm9uZSIpCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04ODEwNQorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gYW4gYXVkaW8gZWxlbWVudCBoYXMg bm8gY29udHJvbHMgYXR0cmlidXRlLCBpdCBzaG91bGQgbm90IGJlIHZpc2libGUgb24gdGhlIHBh Z2UuCisgICAgICAgIEJlZm9yZSB0aGlzIHBhdGNoLCB3aGVuIHdlIHJlbW92ZWQgdGhlICJjb250 cm9scyIgYXR0cmlidXRlLCBhbiBlbXB0eSBkaXYgb2YgdGhlCisgICAgICAgIHNpemUgb2YgdGhl IGNvbnRyb2xzIHJlbWFpbmVkLgorCisgICAgICAgICogbWVkaWEvYXVkaW8tY29udHJvbHMtaGlk ZGVuLWRpc3BsYXktbm9uZS1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIG1lZGlhL2F1 ZGlvLWNvbnRyb2xzLWhpZGRlbi1kaXNwbGF5LW5vbmUuaHRtbDogQWRkZWQuCisKIDIwMTItMDgt MjkgIE5vZWwgR29yZG9uICA8bm9lbC5nb3Jkb25AZ21haWwuY29tPgogCiAgICAgICAgIFdlYlBJ bWFnZURlY29kZXIgcHJvZ3Jlc3NpdmUgZGVjb2RlcyBmYWlsIHRvIGRlY29kZSB2YWxpZCBpbWFn ZXMKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL21lZGlhL2F1ZGlvLWNvbnRyb2xzLWhpZGRlbi1k aXNwbGF5LW5vbmUtZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMvbWVkaWEvYXVkaW8tY29udHJv bHMtaGlkZGVuLWRpc3BsYXktbm9uZS1leHBlY3RlZC50eHQKbmV3IGZpbGUgbW9kZSAxMDA2NDQK aW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uMjA4ZmUwNGJh YmY1ZTI4YjMxMDM3ZTczNjc3ZjU3MzU5ZmI3M2U0NAotLS0gL2Rldi9udWxsCisrKyBiL0xheW91 dFRlc3RzL21lZGlhL2F1ZGlvLWNvbnRyb2xzLWhpZGRlbi1kaXNwbGF5LW5vbmUtZXhwZWN0ZWQu dHh0CkBAIC0wLDAgKzEsMTAgQEAKK1RoaXMgdGVzdHMgaWYgYW4gYXVkaW8gZmlsZSB0aGF0IGhh cyBubyBjb250cm9scyBpcyBhbHdheXMgZGlzcGxheTogbm9uZSwgaW5jbHVkaW5nIGFmdGVyIHNj cmlwdCByZW1vdmFsIG9mIGF0dHJpYnV0ZS4KKworRVhQRUNURUQgKGdldENvbXB1dGVkU3R5bGUo YXVkaW8pWydkaXNwbGF5J10gPT0gJ25vbmUnKSBPSworCitFWFBFQ1RFRCAoZ2V0Q29tcHV0ZWRT dHlsZShhdWRpbylbJ2Rpc3BsYXknXSA9PSAnaW5saW5lJykgT0sKKworRVhQRUNURUQgKGdldENv bXB1dGVkU3R5bGUoYXVkaW8pWydkaXNwbGF5J10gPT0gJ25vbmUnKSBPSworCitFTkQgT0YgVEVT VAorCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9tZWRpYS9hdWRpby1jb250cm9scy1oaWRkZW4t ZGlzcGxheS1ub25lLmh0bWwgYi9MYXlvdXRUZXN0cy9tZWRpYS9hdWRpby1jb250cm9scy1oaWRk ZW4tZGlzcGxheS1ub25lLmh0bWwKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uOTM1N2Y0MTFhMzJjNzFjMmEyMTFlNjM2 MmJkYTY0MTk4YTI3ZmQ4ZgotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL21lZGlhL2F1 ZGlvLWNvbnRyb2xzLWhpZGRlbi1kaXNwbGF5LW5vbmUuaHRtbApAQCAtMCwwICsxLDQ1IEBACis8 IURPQ1RZUEUgaHRtbD4KKzxodG1sPgorPGhlYWQ+CisgICAgPHRpdGxlPkF1ZGlvIENvbnRyb2xz IC0gc2VsZWN0aXZlIGNvbnRyb2wgZWxlbWVudCBkaXNwbGF5PC90aXRsZT4KKyAgICA8c2NyaXB0 IHNyYz0idmlkZW8tdGVzdC5qcyI+PC9zY3JpcHQ+CisgICAgPHNjcmlwdCBzcmM9Im1lZGlhLWZp bGUuanMiPjwvc2NyaXB0PgorICAgIDxzY3JpcHQ+CisgICAgdmFyIGF1ZGlvOworCisgICAgZnVu Y3Rpb24gdGVzdERpc3BsYXkoZXZlbnQpCisgICAgeworICAgICAgICBhdWRpbyA9IGV2ZW50LnRh cmdldDsKKworICAgICAgICB0ZXN0RXhwZWN0ZWQoImdldENvbXB1dGVkU3R5bGUoYXVkaW8pWydk aXNwbGF5J10iLCAnbm9uZScsICI9PSIpOworICAgICAgICBjb25zb2xlV3JpdGUoIiIpOworCisg ICAgICAgIGF1ZGlvLnNldEF0dHJpYnV0ZSgiY29udHJvbHMiLCAiIik7CisKKyAgICAgICAgdGVz dEV4cGVjdGVkKCJnZXRDb21wdXRlZFN0eWxlKGF1ZGlvKVsnZGlzcGxheSddIiwgJ2lubGluZScs ICI9PSIpOworICAgICAgICBjb25zb2xlV3JpdGUoIiIpOworCisgICAgICAgIGF1ZGlvLnJlbW92 ZUF0dHJpYnV0ZSgiY29udHJvbHMiKTsKKworICAgICAgICB0ZXN0RXhwZWN0ZWQoImdldENvbXB1 dGVkU3R5bGUoYXVkaW8pWydkaXNwbGF5J10iLCAnbm9uZScsICI9PSIpOworICAgICAgICBjb25z b2xlV3JpdGUoIiIpOworCisgICAgICAgIGVuZFRlc3QoKTsKKyAgICB9CisKKyAgICBmdW5jdGlv biBpbml0KCkKKyAgICB7CisgICAgICAgIGF1ZGlvID0gZG9jdW1lbnQuZ2V0RWxlbWVudHNCeVRh Z05hbWUoImF1ZGlvIilbMF07CisgICAgICAgIGF1ZGlvLmFkZEV2ZW50TGlzdGVuZXIoJ2NhbnBs YXl0aHJvdWdoJywgdGVzdERpc3BsYXksIGZhbHNlKTsKKyAgICAgICAgYXVkaW8uc3JjID0gZmlu ZE1lZGlhRmlsZSgnYXVkaW8nLCAnY29udGVudC9zaG9ydCcpOworICAgIH0KKyAgICA8L3Njcmlw dD4KKzwvaGVhZD4KKzxib2R5IG9ubG9hZD0iaW5pdCgpIj4KKyAgICA8YXVkaW8+PC9hdWRpbz4K KyAgICA8cD4KKyAgICAgICAgVGhpcyB0ZXN0cyBpZiBhbiBhdWRpbyBmaWxlIHRoYXQgaGFzIG5v IGNvbnRyb2xzIGlzIGFsd2F5cyBkaXNwbGF5OiBub25lLAorICAgICAgICBpbmNsdWRpbmcgYWZ0 ZXIgc2NyaXB0IHJlbW92YWwgb2YgYXR0cmlidXRlLgorICAgIDwvcD4KKzwvYm9keT4KKzwvaHRt bD4K