Bug 105839 - [HTMLTemplateElement] Allow <template> content to be inspected
Summary: [HTMLTemplateElement] Allow <template> content to be inspected
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rafael Weinstein
URL:
Keywords:
Depends on:
Blocks: 103547
  Show dependency treegraph
 
Reported: 2012-12-28 14:34 PST by Rafael Weinstein
Modified: 2013-01-08 16:44 PST (History)
17 users (show)

See Also:


Attachments
Patch (5.45 KB, patch)
2012-12-28 14:36 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch (11.10 KB, patch)
2013-01-02 17:39 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch (11.13 KB, patch)
2013-01-02 20:45 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch (11.20 KB, patch)
2013-01-03 09:48 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch (12.77 KB, patch)
2013-01-04 15:04 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch for landing (13.03 KB, patch)
2013-01-08 14:59 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rafael Weinstein 2012-12-28 14:34:55 PST
<template> should be expandable in the 'elements' tab and it should contain a single '#document-fragment' child whose children are expandable.
Comment 1 Rafael Weinstein 2012-12-28 14:36:51 PST
Created attachment 180901 [details]
Patch
Comment 2 Rafael Weinstein 2012-12-28 14:40:09 PST
Here is the start of a patch for this. The main problem is that when anything in the templateContent changes, it is not updated in the UI.

I believe this is because there isn't an associated instrumenting agent with the owner document of the content or its children.

Pavel, a little background: The template element's content document fragment is "inert" (images don't load, script doesn't run, etc...). The way this is accomplished is that it is owned by a different document which doesn't have a browsing context (frame).

Unfortunately. this means that instrumentingAgentsForDocument() currently always returns 0;

What's the best way to approach this?
Comment 3 Early Warning System Bot 2012-12-28 14:50:33 PST
Comment on attachment 180901 [details]
Patch

Attachment 180901 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15549924
Comment 4 Early Warning System Bot 2012-12-28 14:51:44 PST
Comment on attachment 180901 [details]
Patch

Attachment 180901 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15559831
Comment 5 EFL EWS Bot 2012-12-28 14:54:56 PST
Comment on attachment 180901 [details]
Patch

Attachment 180901 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15587019
Comment 6 Build Bot 2012-12-28 15:27:15 PST
Comment on attachment 180901 [details]
Patch

Attachment 180901 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15550992
Comment 7 Pavel Feldman 2012-12-29 03:05:25 PST
Comment on attachment 180901 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180901&action=review

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1300
> +        if (element->hasTagName(HTMLNames::templateTag))

#if ENABLE(TEMPLATE_ELEMENT)

> Source/WebCore/inspector/front-end/DOMAgent.js:78
> +    if (payload.templateContent) {

No need for {} around one line block.
Comment 8 Pavel Feldman 2012-12-29 03:16:17 PST
> Pavel, a little background: The template element's content document fragment is "inert" (images don't load, script doesn't run, etc...). The way this is accomplished is that it is owned by a different document which doesn't have a browsing context (frame).



> 
> Unfortunately. this means that instrumentingAgentsForDocument() currently always returns 0;
> 
> What's the best way to approach this?

Could we do something like

m_templateContentsOwnerDocument->setTemplateContentsOwnerDocumentParent(this);

in modify Document::templateContentsOwnerDocument()?

Then instrumentingAgentsForDocument would follow that link and fetch appropriate page.
Comment 9 Rafael Weinstein 2013-01-02 17:36:28 PST
Comment on attachment 180901 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180901&action=review

>> Source/WebCore/inspector/InspectorDOMAgent.cpp:1300
>> +        if (element->hasTagName(HTMLNames::templateTag))
> 
> #if ENABLE(TEMPLATE_ELEMENT)

done

>> Source/WebCore/inspector/front-end/DOMAgent.js:78
>> +    if (payload.templateContent) {
> 
> No need for {} around one line block.

done
Comment 10 Rafael Weinstein 2013-01-02 17:39:00 PST
Created attachment 181119 [details]
Patch
Comment 11 Rafael Weinstein 2013-01-02 17:40:36 PST
Ok, trying Pavel's suggestion.
Comment 12 Early Warning System Bot 2013-01-02 17:50:51 PST
Comment on attachment 181119 [details]
Patch

Attachment 181119 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15625750
Comment 13 Early Warning System Bot 2013-01-02 17:52:29 PST
Comment on attachment 181119 [details]
Patch

Attachment 181119 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15636645
Comment 14 Build Bot 2013-01-02 18:47:43 PST
Comment on attachment 181119 [details]
Patch

Attachment 181119 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15630745
Comment 15 Build Bot 2013-01-02 19:24:42 PST
Comment on attachment 181119 [details]
Patch

Attachment 181119 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15628765
Comment 16 Rafael Weinstein 2013-01-02 20:45:32 PST
Created attachment 181141 [details]
Patch
Comment 17 Pavel Feldman 2013-01-03 00:46:32 PST
Comment on attachment 181141 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181141&action=review

> Source/WebCore/dom/Document.cpp:616
> +        m_inertDocument.clear();

This will happen on its own.

> Source/WebCore/dom/Document.cpp:5949
> +Document* Document::inertDocument()

I don't find the new name self-explanatory, I'd rather leave it as is. r- is for this, otherwise lgtm.
Comment 18 Rafael Weinstein 2013-01-03 06:48:09 PST
Comment on attachment 181141 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181141&action=review

>> Source/WebCore/dom/Document.cpp:616
>> +        m_inertDocument.clear();
> 
> This will happen on its own.

done

>> Source/WebCore/dom/Document.cpp:5949
>> +Document* Document::inertDocument()
> 
> I don't find the new name self-explanatory, I'd rather leave it as is. r- is for this, otherwise lgtm.

I renamed this one to avoid having to call the back pointer "templateContentsOwnerDocumentOwner" (what is currently "inertDocumentCreator").

I suppose I could go with "templateContentsOwner" and "templateContentsOwnerCreator"? ("Document" is implied in the type).

Thoughts?
Comment 19 Rafael Weinstein 2013-01-03 09:48:41 PST
Created attachment 181180 [details]
Patch
Comment 20 Rafael Weinstein 2013-01-03 09:48:55 PST
How ya like me now?
Comment 21 Rafael Weinstein 2013-01-04 15:04:57 PST
Created attachment 181384 [details]
Patch
Comment 22 Rafael Weinstein 2013-01-04 15:05:19 PST
New patch resolves merge conflicts.

Pavel: ping?
Comment 23 Rafael Weinstein 2013-01-08 14:59:34 PST
Created attachment 181776 [details]
Patch for landing
Comment 24 WebKit Review Bot 2013-01-08 16:44:14 PST
Comment on attachment 181776 [details]
Patch for landing

Clearing flags on attachment: 181776

Committed r139132: <http://trac.webkit.org/changeset/139132>
Comment 25 WebKit Review Bot 2013-01-08 16:44:19 PST
All reviewed patches have been landed.  Closing bug.