1539265 - expose ::marker pseudo-element in inspector

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Bill Goldstein [:aja] (UTC-6)]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

select a <li> or a <summary> element in inspector

Actual results:

no ::marker pseudo-element is exposed under the element

Expected results:

::marker pseudo-element should be exposed under the element

Comment 1

[Ian Moody [:Kwan] (UTC+1)]

Note: this is currently exposable by toggling devtools.inspector.showAllAnonymousContent to true in about:config, but it displays with the raw <_moz_generated_content_marker></_moz_generated_content_marker> appearance. Fixing it to display as ::marker would be pretty easy given the existing code for doing the same to ::before and ::after, but do we actually want to show it all the time for all <li> & <summary> elements? Or would that be too noisy?

Also at the moment dragging & dropping a node before ::markers leads to crashes:
bp-2fc640fe-43fa-4454-a555-ef02e0190327
That needs handling here:
https://searchfox.org/mozilla-central/rev/a7315d78417179b151fef6108f2bce14786ba64d/devtools/client/inspector/markup/markup.js#2161-2197

Comment 2

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Handle ::marker pseudo-elements on shadow roots in walker.js.

At the moment when devtools.inspector.showAllAnonymousContent is true shadow
hosts with ::marker and ::before pseudos will stop showing the ::before and
won't show the ::marker either.

Comment 3

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Add a test for ::marker displaying in the markup view.

Comment 4

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Handle ::marker pseudo-elements in css-logic.js, so their rules show in the rule view.

Comment 5

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Add isMarkerPseudoElement function to layout/utils.js.

Comment 6

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Handle ::marker pseudos during drag & drop of nodes in the markup view.

Prior to this the tab would crash with signature [@ nsINode::InsertChildBefore ]
if trying to insert before the ::marker.

Comment 7

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Add a few optimisations for ::marker pseudos.

Where there is existing for ::before and ::after.

Comment 8

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Don't represent ::marker pseudo-elements with the internal _moz_generated_content_marker tagName in the markup view.

Comment 9

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Add ::marker pseudo-element handling to WalkerIndex.

So ::marker pseudo elements can be found via searching for "::marker" in the
markup view.

Comment 10

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Add ::marker to the list of suggested pseudo selectors.

For autocompletion of : in selector contexts, like the Style Editor.

Comment 11

[Ian Moody [:Kwan] (UTC+1)]

So the current stack gets it into a good state where after toggling the anon pref:

• ::befores on shadow hosts don't disappear if the host as a ::marker as well
• rules are shown in the rule view when a ::marker is selected
• the tab doesn't crash when drag & dropping a node before the ::marker
• ::markers are displayed in the markup view with the proper pseudo representation instead of the internal generated content tag name (and in the breadcrumbs)
• ::markers can be found via searching "::marker" in the markup view

I still need to add more tests, especially one for the drag & drop crash.
Then we need to decide if we should always show markers, or only after a pref-flip (presumably a new pref, separate to the anon one, perhaps even with UI)

Comment 12

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Handle ::marker pseudo-elements in reps.

Though I don't currently understand how those functions are used or how to test
them.

Comment 13

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1539265 - Make ::marker pseudo-elements always visible in the markup view.

Instead of only when setting devtools.inspector.showAllAnonymousContent.

Comment 14

[Ian Moody [:Kwan] (UTC+1)]

Try builds available for experimentation here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a21727618d5244ed2d93f834b63afad7dd839d

These are to the top of the patch stack, so ::markers display by default. Please test it out and report any feedback, any noticed bugs, and whether or not always having ::markers display creates too much noise or seems fine.

Comment 15

[Bill Goldstein [:aja] (UTC-6)]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #14)

Try builds available for experimentation here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a21727618d5244ed2d93f834b63afad7dd839d

These are to the top of the patch stack, so ::markers display by default. Please test it out and report any feedback, any noticed bugs, and whether or not always having ::markers display creates too much noise or seems fine.

Thanks, looks good to me after testing the win64 Try build. Will update here if I spot anything odd.
Not too noisy as far as the reporter is concerned.

Comment 16

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Before and After of simple list elements in markup view

So one immediate disadvantage is that list items whose contents are just a short text node will no longer be shown inline. Attachment shows the difference on https://developer.mozilla.org/en-US/docs/Web/HTML/Element/li

Comment 17

[Bill Goldstein [:aja] (UTC-6)]

I suppose you could, in the case of short text node, display the ellipsis button before(in LTR)/after(in RTL) the short text run.
Seems to me like a lot of extra work for little gain, but I guess it'd be doable.

Comment 18

[Ian Moody [:Kwan] (UTC+1)]

A surprisingly-green try here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=74b34fa91e8893a11f0c8cc069a2c4839b862335

(I'm sure something will go wrong on landing to make up for it)

Comment 19

[Cosmin Sabou [:CosminS]]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmptNRphF\npatching file devtools/client/shared/components/reps/reps.js\nHunk #1 FAILED at 1115\nHunk #2 FAILED at 1238\nHunk #3 FAILED at 1353\nHunk #4 FAILED at 1485\nHunk #5 FAILED at 1532\nHunk #6 FAILED at 1575\nHunk #7 FAILED at 1625\nHunk #8 succeeded at 2453 with fuzz 2 (offset 361 lines).\nHunk #18 FAILED at 2856\nHunk #20 FAILED at 2961\nHunk #30 FAILED at 3731\nHunk #31 FAILED at 3769\nHunk #32 FAILED at 3781\nHunk #33 FAILED at 4064\nHunk #34 FAILED at 4074\nHunk #35 FAILED at 4185\nHunk #36 FAILED at 4217\nHunk #37 FAILED at 4431\nHunk #38 FAILED at 4889\nHunk #40 FAILED at 5078\nHunk #41 FAILED at 5143\nHunk #42 FAILED at 5165\nHunk #43 FAILED at 5180\nHunk #44 FAILED at 5470\nHunk #45 FAILED at 5481\nHunk #46 FAILED at 6348\nHunk #47 FAILED at 6517\nHunk #48 FAILED at 6529\nHunk #49 FAILED at 7093\nHunk #50 FAILED at 7314\nHunk #51 FAILED at 7375\nHunk #52 FAILED at 7566\n31 out of 52 hunks FAILED -- saving rejects to file devtools/client/shared/components/reps/reps.js.rej\nabort: patch failed to apply', '')

Comment 20

[Ian Moody [:Kwan] (UTC+1)]

Clash with bug 1540239 which also touched the auto-generated devtools/client/shared/components/reps/reps.js. Auto rebase was never going to handle that. Rebased and re-generated devtools/client/shared/components/reps/reps.js.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/931e98129878
Handle ::marker pseudo-elements on shadow roots in walker.js. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/72bbb4b28932
Add a test for ::marker displaying in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/637a9d6af0f3
Handle ::marker pseudo-elements in css-logic.js, so their rules show in the rule view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/19c8e404ac5f
Add isMarkerPseudoElement function to layout/utils.js. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/8015aeb5279d
Handle ::marker pseudos during drag & drop of nodes in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/3add7d4d4853
Add a few optimisations for ::marker pseudos. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/6eaf5bdd945c
Don't represent ::marker pseudo-elements with the internal _moz_generated_content_marker tagName in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/18fa7e971a0d
Add ::marker pseudo-element handling to WalkerIndex. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/f78146b8aa17
Add ::marker to the list of suggested pseudo selectors. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/be9f5f73bdfe
Handle ::marker pseudo-elements in reps. r=jdescottes,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/8206b880f9ec
Make ::marker pseudo-elements always visible in the markup view. r=jdescottes,mbalfanz,pbro

Comment 22

[Arthur Iakab [arthur_iakab]]

Backed out 11 changesets (bug 1539265) for causing debugger failures CLOSED TREE

Bsckout revision https://hg.mozilla.org/integration/autoland/rev/85c0179c84be93c568c778a30de909ed8c9feff5

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=8206b880f9ecf19c5933efe72addf9c350977199&selectedJob=237457533&searchStr=linux%2Cx64%2Copt%2Cnode%2Ctests%2Csource-test-node-debugger-tests%2Cnode%28debugger%29

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=237457533&repo=autoland

:Kwan can you please take a look?

Comment 23

[Ian Moody [:Kwan] (UTC+1)]

Ah, missed that test since I was only doing try fuzzy 'linux64 'devtools. Fortunately found the inbuilt try --preset debugger-tests now.

Green try (with one bug 1507337 orange, and shippable-related artifact bustages):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fdbd270e5a2baf9ec8e15885c481b5d52b9d9769&selectedJob=237466073

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e17935240c9
Handle ::marker pseudo-elements on shadow roots in walker.js. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/ed3ec6d0ecb2
Add a test for ::marker displaying in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/15349b80d230
Handle ::marker pseudo-elements in css-logic.js, so their rules show in the rule view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/f65cad0863f9
Add isMarkerPseudoElement function to layout/utils.js. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/25c6f5583502
Handle ::marker pseudos during drag & drop of nodes in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/c4a552cb33e3
Add a few optimisations for ::marker pseudos. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/ad3465ab22b6
Don't represent ::marker pseudo-elements with the internal _moz_generated_content_marker tagName in the markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/0b3e62a77748
Add ::marker pseudo-element handling to WalkerIndex. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/98af58faaa11
Add ::marker to the list of suggested pseudo selectors. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/32d6fbf8449a
Handle ::marker pseudo-elements in reps. r=jdescottes,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fb4a79e4b5f9
Make ::marker pseudo-elements always visible in the markup view. r=jdescottes,mbalfanz,pbro

Comment 25

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1e17935240c9
https://hg.mozilla.org/mozilla-central/rev/ed3ec6d0ecb2
https://hg.mozilla.org/mozilla-central/rev/15349b80d230
https://hg.mozilla.org/mozilla-central/rev/f65cad0863f9
https://hg.mozilla.org/mozilla-central/rev/25c6f5583502
https://hg.mozilla.org/mozilla-central/rev/c4a552cb33e3
https://hg.mozilla.org/mozilla-central/rev/ad3465ab22b6
https://hg.mozilla.org/mozilla-central/rev/0b3e62a77748
https://hg.mozilla.org/mozilla-central/rev/98af58faaa11
https://hg.mozilla.org/mozilla-central/rev/32d6fbf8449a
https://hg.mozilla.org/mozilla-central/rev/fb4a79e4b5f9