Bug 26315 - Empty element's ending tag is not shown by inspector
Summary: Empty element's ending tag is not shown by inspector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
: 17071 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-06-11 04:55 PDT by Pavel Feldman
Modified: 2010-03-12 07:11 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed solution (5.22 KB, patch)
2010-02-17 02:27 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[IMAGE] Elements panel showing closing tags (with the patch applied) (39.68 KB, image/png)
2010-02-17 02:30 PST, Alexander Pavlov (apavlov)
no flags Details
[PATCH] Comments addressed (6.03 KB, patch)
2010-02-17 04:37 PST, Alexander Pavlov (apavlov)
timothy: review-
Details | Formatted Diff | Diff
[IMAGE] Fixed "..." color and removed closing tags for elements that forbid them (46.79 KB, image/png)
2010-02-17 04:40 PST, Alexander Pavlov (apavlov)
no flags Details
[PATCH] Closing tags always rendered for XML-type documents, tests re-baselined (12.39 KB, patch)
2010-02-18 01:47 PST, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff
[PATCH] Comments addressed, test for XHTML added (18.11 KB, patch)
2010-02-19 06:48 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***