Issue 6269930580213760: Issue 309 - Don't use Shadow DOM in Chrome 32.0.1700 (Closed)
|
Created:
April 17, 2014, 11:52 a.m. by Sebastian Noack Modified:
April 17, 2014, 1:34 p.m. Visibility:
Public. |
DescriptionEven tough Chrome 32.0.1700 supports Shadow DOM, it ignores the <shadow> element, leading to pages rendered empty.
Patch Set 1 #
Total comments: 5
Patch Set 2 : Fixed typo in comment #MessagesTotal messages: 9
Seems a bit hacky to hard code the exact version here - Thomas, can you think of any other way? I'm not too familiar with shadow DOM. http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... File include.preload.js (right): http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:32: if ("webkitCreateShadowRoot" in document.documentElement && !/\bChrome\/32\.0\.1700\b/.test(navigator.userAgent)) Is there no other way to figure out if the shadow element is being ignored?
http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... File include.preload.js (right): http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:32: if ("webkitCreateShadowRoot" in document.documentElement && !/\bChrome\/32\.0\.1700\b/.test(navigator.userAgent)) On 2014/04/17 12:05:38, Felix H. Dahlke wrote: > Is there no other way to figure out if the shadow element is being ignored? Please see the discussion at https://issues.adblockplus.org/ticket/309. Wladimir suggested to check for presence of HTMLShadowElement. But that object exist and even |document.createElement("shadow") instanceof HTMLShadowElement| returns true. But it is completly ignored when the website is rendered.
http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... File include.preload.js (right): http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:31: // <shadow> element is broken in Chrome Chrome 32.0.1700 (#309) "in Chrome Chrome" ;) http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:32: if ("webkitCreateShadowRoot" in document.documentElement && !/\bChrome\/32\.0\.1700\b/.test(navigator.userAgent)) On 2014/04/17 12:05:38, Felix H. Dahlke wrote: > Is there no other way to figure out if the shadow element is being ignored? In what way is it broken? You mention that the "<shadow> element is ignored". Would the following check help for this case? typeof HTMLShadowElement != "undefined" and/or document.createElement("shadow") instanceof HTMLShadowElement
http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... File include.preload.js (right): http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:31: // <shadow> element is broken in Chrome Chrome 32.0.1700 (#309) On 2014/04/17 12:24:17, Thomas Greiner wrote: > "in Chrome Chrome" ;) Done. http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... include.preload.js:32: if ("webkitCreateShadowRoot" in document.documentElement && !/\bChrome\/32\.0\.1700\b/.test(navigator.userAgent)) On 2014/04/17 12:24:17, Thomas Greiner wrote: > On 2014/04/17 12:05:38, Felix H. Dahlke wrote: > > Is there no other way to figure out if the shadow element is being ignored? > > In what way is it broken? You mention that the "<shadow> element is ignored". Same effect as if it wouldn't be there. And since <shadow> is the insertion point, that means that the page is always rendered empty. > Would the following check help for this case? > > typeof HTMLShadowElement != "undefined" > > and/or > > document.createElement("shadow") instanceof HTMLShadowElement No, see my previous comment.
On 2014/04/17 12:30:32, Sebastian Noack wrote: > http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... > File include.preload.js (right): > > http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... > include.preload.js:31: // <shadow> element is broken in Chrome Chrome 32.0.1700 > (#309) > On 2014/04/17 12:24:17, Thomas Greiner wrote: > > "in Chrome Chrome" ;) > > Done. > > http://codereview.adblockplus.org/6269930580213760/diff/5629499534213120/incl... > include.preload.js:32: if ("webkitCreateShadowRoot" in document.documentElement > && !/\bChrome\/32\.0\.1700\b/.test(navigator.userAgent)) > On 2014/04/17 12:24:17, Thomas Greiner wrote: > > On 2014/04/17 12:05:38, Felix H. Dahlke wrote: > > > Is there no other way to figure out if the shadow element is being ignored? > > > > In what way is it broken? You mention that the "<shadow> element is ignored". > > Same effect as if it wouldn't be there. And since <shadow> is the insertion > point, that means that the page is always rendered empty. > > > Would the following check help for this case? > > > > typeof HTMLShadowElement != "undefined" > > > > and/or > > > > document.createElement("shadow") instanceof HTMLShadowElement > > No, see my previous comment. If nothing Thomas suggested works, can you mention this in the comment? Would like to avoid any WTF moments for this exact version check, should be clear what we tried and why it's the only way.
On 2014/04/17 12:50:14, Felix H. Dahlke wrote: > If nothing Thomas suggested works, can you mention this in the comment? Would > like to avoid any WTF moments for this exact version check, should be clear what > we tried and why it's the only way. The comment already says that <shadow> is broken, as opposed to missing which would be trivial to check for. Also the comment refers to the issue, where you can read all the details.
On 2014/04/17 13:06:51, Sebastian Noack wrote: > On 2014/04/17 12:50:14, Felix H. Dahlke wrote: > > If nothing Thomas suggested works, can you mention this in the comment? Would > > like to avoid any WTF moments for this exact version check, should be clear > what > > we tried and why it's the only way. > > The comment already says that <shadow> is broken, as opposed to missing which > would be trivial to check for. Also the comment refers to the issue, where you > can read all the details. Oh you're right, didn't realise it was thoroughly discussed in that issues. LGTM from me then. Please make Thomas a reviewer and wait for his LGTM too - he's more into this topic.
Seems like everything we could use for this case is not yet implemented so LGTM. 
|
