After Bug 1449333 we now support both open and closed shadow roots. The #shadow-root node in the markup view could add a visual indicator to show if the shadow root is open or closed. For instance chrome devtools add "(open)" or "(closed)" after #shadow-root.

This is probably another good bug for shadow dom work. We can start simply with adding (open) or (closed) next to #shadow-root in the markup view. The markup for #shadow-root elements is created in the read-only-editor. The read-only-editor is on the client side, and therefore has access to nodeFront objects. A front represents a server side object giving you access to some serialized properties and methods. Look at https://docs.firefox-dev.tools/backend/protocol.js.html for more details. The most important to remember in the scope of this bug is that the object returned by NodeActor's form() method is serialized and accessible by the NodeFront at this._form. So here you will have to add a new information in the NodeActor form (eg isClosedShadowRoot?), maybe add a small function in the NodeFront to read this from _form, and finally use this information in read-only-editor.js node actor: https://searchfox.org/mozilla-central/source/devtools/server/actors/inspector/node.js node front: https://searchfox.org/mozilla-central/source/devtools/shared/fronts/node.js read-only-editor: https://searchfox.org/mozilla-central/source/devtools/client/inspector/markup/views/read-only-editor.js For tests, you can have a look at anything in https://searchfox.org/mozilla-central/search?q=&case=true&regexp=false&path=browser_markup_shadow.

Comment on attachment 8991629 [details] Bug 1473901 - show shadow root mode. https://reviewboard.mozilla.org/r/256552/#review263410 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: devtools/client/inspector/markup/views/read-only-editor.js:15 (Diff revision 1) > + new LocalizationHelper("devtools/client/locales/inspector.properties"); > + > +const SHADOW_MODES_L10N = { > + open: INSPECTOR_L10N.getStr("markupView.shadowDOM.mode.open"), > + closed: INSPECTOR_L10N.getStr("markupView.shadowDOM.mode.closed"), > +} Error: Missing semicolon. [eslint: semi]

Comment on attachment 8991629 [details] Bug 1473901 - show shadow root mode. https://reviewboard.mozilla.org/r/256552/#review263560 ::: devtools/client/locales/en-US/inspector.properties:42 (Diff revision 2) > # Used in a tooltip that appears when the user hovers over whitespace-only text nodes in > # the inspector. > markupView.whitespaceOnly=Whitespace-only text node: %S > > +# LOCALIZATION NOTE > +# Used to indicate the mode of a shadow root node (open or closed) I'm wondering if we want to add a note *not* to localize this string, since 'open' and 'closed' is defined by the API: https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/mode. In fact, I wonder if we should even add this as a string in the properties file vs directly setting it as textContent in the UI. Going to needinfo l10n for guidance here.

(In reply to Brian Grinstead [:bgrins] from comment #5) > Comment on attachment 8991629 [details] > Bug 1473901 - show shadow root mode. > > https://reviewboard.mozilla.org/r/256552/#review263560 > > ::: devtools/client/locales/en-US/inspector.properties:42 > (Diff revision 2) > > # Used in a tooltip that appears when the user hovers over whitespace-only text nodes in > > # the inspector. > > markupView.whitespaceOnly=Whitespace-only text node: %S > > > > +# LOCALIZATION NOTE > > +# Used to indicate the mode of a shadow root node (open or closed) > > I'm wondering if we want to add a note *not* to localize this string, since > 'open' and 'closed' is defined by the API: > https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/mode. > > In fact, I wonder if we should even add this as a string in the properties > file vs directly setting it as textContent in the UI. Going to needinfo l10n > for guidance here. Axel - since :flod is away, would you be able to weigh in here or redirect?

Comment on attachment 8991629 [details] Bug 1473901 - show shadow root mode. https://reviewboard.mozilla.org/r/256552/#review263560 > I'm wondering if we want to add a note *not* to localize this string, since 'open' and 'closed' is defined by the API: https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot/mode. > > In fact, I wonder if we should even add this as a string in the properties file vs directly setting it as textContent in the UI. Going to needinfo l10n for guidance here. I was wondering the same, since I'm not sure if we are showing the API value or we are showing a "human-friendly interpretation". If we go to show the API value straight away, I also agree to remove the strings from the localization file.

Comment on attachment 8991629 [details] Bug 1473901 - show shadow root mode. https://reviewboard.mozilla.org/r/256552/#review263632 I'm torn on wether to localize the shadow-root or not. I've looked at MDN localizations, and there's not a lot yet. But at least some keep 'shadow root' intact, so maybe it's a good idea to do that in the UI, too. Do you have any general policies on that? I left some notes on general problems with hard-coded strings in this patch for educational purposes only ;-) ::: devtools/client/inspector/markup/views/read-only-editor.js:35 (Diff revision 2) > } else if (node.nodeType == nodeConstants.DOCUMENT_TYPE_NODE) { > this.elt.classList.add("comment", "doctype"); > this.tag.textContent = node.doctypeString; > } else if (node.isShadowRoot) { > - this.tag.textContent = "#shadow-root"; > + this.tag.textContent = "#shadow-root" + > + ` (${SHADOW_MODES_L10N[node.shadowRootMode]})`; This is hard-coded UI text: " (" and ")". This should be part of the localized string. Best way would probably be to expose both #shadow-root (closed) #shadow-root (open) as two separate strings. Or not at all ;-). ::: devtools/client/locales/en-US/inspector.properties:42 (Diff revision 2) > # Used in a tooltip that appears when the user hovers over whitespace-only text nodes in > # the inspector. > markupView.whitespaceOnly=Whitespace-only text node: %S > > +# LOCALIZATION NOTE > +# Used to indicate the mode of a shadow root node (open or closed) Ride along note, the localization comment here isn't going to make it to the second string, pontoon will only show it on the first.

Hi, after speaking with Julian, we both agree that `open` and `closed` are API values and shouldn't be localized –as Brian suggested. So I have removed the strings from the localization file and they are now hardcoded instead as the `textContent` of the node. Thanks!

Axel, are you OK with this not being localized? Thanks :)

(In reply to Belén [:ladybenko] from comment #11) > Axel, are you OK with this not being localized? Thanks :) Yes, it's OK to keep it hard-coded in this case.

Fixed eslint errors in the test file

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 01eece0d8ffccd9a22bbcbb58ae65414757606df -d 050d2e0e3b62: rebasing 473464:01eece0d8ffc "Bug 1473901 - show shadow root mode. r=bgrins" (tip) merging devtools/client/inspector/markup/test/browser.ini merging devtools/server/actors/inspector/node.js warning: conflicts while merging devtools/client/inspector/markup/test/browser.ini! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by balbeza@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98f850870874 show shadow root mode. r=bgrins

I have reproduced this issue using Firefox 63.0a1 (2018.07.06) on Ubuntu 16.04 x86. I can confirm this issue is fixed, I verified using Firefox 63.0b5 on Ubuntu 16.04 x86, Windows 10 x64 and Mac OS X 10.13.