1269527 - markupview: expanding a #document element displays a duplicated </html> closing tag

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Julian Descottes [:jdescottes]]

Attachment: html-closing-tag.png

STRs: 
1. open data:text/html,<iframe>
2. open the inspector, inspect the iframe and expand #document

AR: The expanded #document should not have a </html> closing tag
ER: has a </html> closing tag (see screenshot)

In markup.js , when expanding a node, the closing line is created by cloning the first ".close" element inside the expanded element:

> 
> let closingTag = this.elt.querySelector(".close");
> // ...
> line.appendChild(closingTag.cloneNode(true));
> 

In case of the #document element, it has no inline ".close" element, so we pick the .close item that belongs to the first child of "#document" -> html.

Comment 1

[Patrick Brosset <:pbro>]

Note that bug 969818 aims at removing the #document node altogether.
So if we do do remove it, then this bug becomes obsolete I guess.

Comment 2

[Brian Grinstead [:bgrins]]

This also appears to affect shadow roots

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1269527 - stop showing incorrect closing tagline for #document;

Review commit: https://reviewboard.mozilla.org/r/215754/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215754/

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8945616 [details]
Bug 1269527 - stop showing incorrect closing tagline for #document;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215754/diff/1-2/

Comment 5

[Julian Descottes [:jdescottes]]

try https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab48a5010c4872df429ec7a5105e0f52be1e8026

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8945616 [details]
Bug 1269527 - stop showing incorrect closing tagline for #document;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215754/diff/2-3/

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8945616 [details]
Bug 1269527 - stop showing incorrect closing tagline for #document;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215754/diff/3-4/

Comment 8

[Julian Descottes [:jdescottes]]

Better try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c0cc71b1c83cb2743709c1e3d3e9f84ec3a6b11

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8945616 [details]
Bug 1269527 - stop showing incorrect closing tagline for #document;

https://reviewboard.mozilla.org/r/215754/#review221814

Thanks, LGTM

::: devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js:18
(Diff revision 4)
> +<iframe src="data:text/html;charset=utf8,<div>test</div>"></iframe>`;
> +
> +add_task(async function () {
> +  let {inspector} = await openInspectorForURL(TEST_URL);
> +
> +  info("Getting the container for the UL parent element");

Nit: "UL" -> "DIV" or ".outer-div"

::: devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js:21
(Diff revision 4)
> +  let {inspector} = await openInspectorForURL(TEST_URL);
> +
> +  info("Getting the container for the UL parent element");
> +  let container = await getContainerForSelector(".outer-div", inspector);
> +  await expandContainer(inspector, container);
> +  ok(container.closeTagLine, "DIV element has a close tag-line");

Nit: should this be `container.closeTagLine && container.closeTagLine.textContent.includes("div")`? If the first assertion fails here we will throw on the next line which means the rest of the test won't complete. IMO it'd be better to continue with the test and see what other failures happen. But I'm OK either way

::: devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js:28
(Diff revision 4)
> +    "close tag-line has the correct content");
> +
> +  info("Expand the iframe element");
> +  container = await getContainerForSelector("iframe", inspector);
> +  await expandContainer(inspector, container);
> +  ok(container.expanded, "#document is expanded");

"iframe is expanded", right?

::: devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js:30
(Diff revision 4)
> +  info("Expand the iframe element");
> +  container = await getContainerForSelector("iframe", inspector);
> +  await expandContainer(inspector, container);
> +  ok(container.expanded, "#document is expanded");
> +  ok(container.closeTagLine, "IFRAME element has a close tag-line");
> +  ok(container.closeTagLine.textContent.includes("iframe"),

Same comment on the assertion for div closeTagLine

::: devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js:50
(Diff revision 4)
> +
> +/**
> + * Expand the provided markup container by clicking on the expand arrow and waiting for
> + * inspector and children to update.
> + */
> +async function expandContainer(inspector, container) {

I'm surprised we don't already have a helper for this

Comment 10

[Julian Descottes [:jdescottes]]

Comment on attachment 8945616 [details]
Bug 1269527 - stop showing incorrect closing tagline for #document;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215754/diff/4-5/

Comment 11

[Julian Descottes [:jdescottes]]

Thanks for the review! Landing.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2b861055936
stop showing incorrect closing tagline for #document;r=bgrins

Comment 13

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/d2b861055936

Comment 14

[Mohammad Maruf Rahman]

I have reproduced this bug with  Nightly 49.0a1 (2016-05-02) on Windows 10, 64 Bit!

This Bug's fix is verified with latest Nightly!

Build ID   : 20180206220102
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Comment 15

[Bogdan Maris, Desktop QA]

(In reply to Mohammad Maruf Rahman from comment #14)
> I have reproduced this bug with  Nightly 49.0a1 (2016-05-02) on Windows 10,
> 64 Bit!
> 
> This Bug's fix is verified with latest Nightly!
> 
> Build ID   : 20180206220102
> User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0)
> Gecko/20100101 Firefox/60.0

Thanks for taking the time to verify this issue.