Bug 26315

Summary: Empty element's ending tag is not shown by inspector
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, commit-queue, eric, timothy
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed solution
none
[IMAGE] Elements panel showing closing tags (with the patch applied)
none
[PATCH] Comments addressed
timothy: review-
[IMAGE] Fixed "..." color and removed closing tags for elements that forbid them
none
[PATCH] Closing tags always rendered for XML-type documents, tests re-baselined
pfeldman: review+
[PATCH] Comments addressed, test for XHTML added none

Description Pavel Feldman 2009-06-11 04:55:48 PDT
1. Open a page with empty script tag in head (<script></script>).
2. Bring up inspector on that page.
3. In head, there is an empty script element

Expected: To show ending script tag
Actual: No end tag will be seen if an element is empty

(Upstreaming chromium bug)
Comment 1 Alexander Pavlov (apavlov) 2010-02-17 02:27:44 PST
Created attachment 48875 [details]
[PATCH] Proposed solution
Comment 2 Alexander Pavlov (apavlov) 2010-02-17 02:30:30 PST
Created attachment 48878 [details]
[IMAGE] Elements panel showing closing tags (with the patch applied)
Comment 3 Timothy Hatcher 2010-02-17 02:38:08 PST
Some comments based on the screenshot alone.

The "…" should be black text.

Some elements like img, input and link (there are more I think) do not need close tags, so we should not show them.
Comment 4 Timothy Hatcher 2010-02-17 03:14:35 PST
Look for HTMLTagStatus endTagRequirement() on the various elements in WebCore to know what ones prohibit end tags in HTML.
Comment 5 Alexander Pavlov (apavlov) 2010-02-17 04:36:13 PST
(In reply to comment #3)
> Some comments based on the screenshot alone.
> 
> The "…" should be black text.

Done.

> Some elements like img, input and link (there are more I think) do not need
> close tags, so we should not show them.

Done. Collected empty elements from HTML4 and HTML5 Draft.
Comment 6 Alexander Pavlov (apavlov) 2010-02-17 04:37:40 PST
Created attachment 48893 [details]
[PATCH] Comments addressed
Comment 7 Alexander Pavlov (apavlov) 2010-02-17 04:40:37 PST
Created attachment 48894 [details]
[IMAGE] Fixed "..." color and removed closing tags for elements that forbid them
Comment 8 Timothy Hatcher 2010-02-17 09:43:33 PST
Comment on attachment 48893 [details]
[PATCH] Comments addressed

> +WebInspector.ElementsTreeElement.ForbiddenClosingTagElements = {
> +    "area": 1,
> +    "base": 1,

This should be an array that you call .keySet() on to get a object. That will let you have multiple tags on a line to take less vertical space.

>          this.treeOutline.updateSelection();
> +        this.updateTitle();

You should call updateTitle before updateSelection. Since updateTitle might change the height of the row and updateSelection needs to ccount for the row height.

>          this.treeOutline.updateSelection();
> +        this.updateTitle();

Ditto, you should call updateTitle before updateSelection.
Comment 9 Alexey Proskuryakov 2010-02-17 13:58:28 PST
> Done. Collected empty elements from HTML4 and HTML5 Draft.

Will this only apply to HTML documents? All elements need to be closed in XML documents.
Comment 10 Alexander Pavlov (apavlov) 2010-02-18 01:47:36 PST
Created attachment 48987 [details]
[PATCH] Closing tags always rendered for XML-type documents, tests re-baselined
Comment 11 Pavel Feldman 2010-02-18 07:37:18 PST
Comment on attachment 48987 [details]
[PATCH] Closing tags always rendered for XML-type documents, tests re-baselined

Can we add a test for XHTML?

> +        this.isXMLMimeType = WebInspector.mainResource && WebInspector.mainResource.mimeType && !!WebInspector.mainResource.mimeType.match(/x(?:ht)?ml/i);

_isXMLMimeType.
Comment 12 Alexander Pavlov (apavlov) 2010-02-19 06:48:12 PST
Created attachment 49075 [details]
[PATCH] Comments addressed, test for XHTML added
Comment 13 WebKit Commit Bot 2010-02-19 07:16:16 PST
Comment on attachment 49075 [details]
[PATCH] Comments addressed, test for XHTML added

Clearing flags on attachment: 49075

Committed r55016: <http://trac.webkit.org/changeset/55016>
Comment 14 WebKit Commit Bot 2010-02-19 07:16:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 Alexander Pavlov (apavlov) 2010-03-12 07:11:01 PST
*** Bug 17071 has been marked as a duplicate of this bug. ***