978657 - Provide Debugger 'introductionType' information for <script> element code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jim Blandy :jimb]

JS code introduced by <script> elements doesn't have any introduction type information set, as retrieved by Debugger.Source.prototype.introductionType.

Comment 1

[Jim Blandy :jimb]

Attachment: Provide introductionType information for HTML and XUL <script> elements.

Try push:
https://tbpl.mozilla.org/?tree=Try&rev=4018f37e6e1d

Comment 2

[Jim Blandy :jimb]

This patch still needs tests for xul:script elements, which seem to be handled by a different place in the code: content/xul/content/src/nsXULElement.cpp instead of content/base/src/nsScriptLoader.cpp. I'll try to get to that tomorrow.

Comment 3

[Jim Blandy :jimb]

Attachment: Convert test_Debugger.Source.prototype.introductionType.html to use addTest and runNextTest.

Using the addTest and runNextTest functions from inspector-helpers.js helps separate the tests from each other in a sane way. This patch is meant to have no effect.

It does change runNextTest to catch and report errors... I don't know why the test harness itself isn't reporting them. *GRR*

Comment 4

[Jim Blandy :jimb]

The patch I just attached needs to be applied before the main patch.

People wanting to try out the change should probably just get builds from the Try push linked to in comment 1.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8384419 [details] [diff] [review]
Convert test_Debugger.Source.prototype.introductionType.html to use addTest and runNextTest.

Review of attachment 8384419 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

Comment 6

[Jim Blandy :jimb]

(In reply to Panos Astithas [:past] from comment #5)
> LGTM.

Thanks!

Comment 7

[Jim Blandy :jimb]

The mochitest-other test failures in the try push in comment 1 are the result of my neglecting to update another previously-existing test for the platform's (improved) behavior; so aside from the testing blunder, they're a good thing. I'll revise that test.

Comment 8

[Jim Blandy :jimb]

Revised try:
https://tbpl.mozilla.org/?tree=Try&rev=c977f0e629c0

Comment 9

[Jim Blandy :jimb]

Attachment: Provide introductionType information for HTML and XUL <script> elements.

Okay, this one has tests for XUL scripts, too. Try push:
https://tbpl.mozilla.org/?tree=Try&rev=16ce16ed15a5

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8388863 [details] [diff] [review]
Provide introductionType information for HTML and XUL <script> elements.

>-
>+if (false) {
Am I missing something here?

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I applied the patch and the introductionType is properly set to "scriptElement". The introductionScript is still missing (needed to complete the logic from here: bug 935203 comment 60).
Let me know if I should file a new bug for it.

Honza

Comment 12

[Jim Blandy :jimb]

(In reply to Olli Pettay [:smaug] from comment #10)
> >+if (false) {
> Am I missing something here?

I'm sorry --- that was careless. I'd diked out those tests temporarily.

Thus, the prior try push was thus kind of meaningless; here's a new one:
https://tbpl.mozilla.org/?tree=Try&rev=ff843b39690b

Comment 13

[Jim Blandy :jimb]

Attachment: Provide introductionType information for HTML and XUL <script> elements.

Revised patch.

Comment 14

[Jim Blandy :jimb]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/613ff1b7bffb
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ced81c068926

Comment 15

[Jim Blandy :jimb]

(In reply to Jan Honza Odvarko from comment #11)
> I applied the patch and the introductionType is properly set to
> "scriptElement". The introductionScript is still missing (needed to complete
> the logic from here: bug 935203 comment 60).
> Let me know if I should file a new bug for it.

<script> element code doesn't necessarily have an introductionScript. For example, if a <script> appears directly in the original HTML document, then there is no reasonable JS script that we can say "introduced" the element.

However, <script> element code should always have an associated 'element'. Could we synthesize names for them by looking at ownerDocument.location.href (in a content-distrustful way, of course)?

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/613ff1b7bffb
https://hg.mozilla.org/mozilla-central/rev/ced81c068926

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jim Blandy :jimb from comment #15)
> (In reply to Jan Honza Odvarko from comment #11)
> > I applied the patch and the introductionType is properly set to
> > "scriptElement". The introductionScript is still missing (needed to complete
> > the logic from here: bug 935203 comment 60).
> > Let me know if I should file a new bug for it.
> 
> <script> element code doesn't necessarily have an introductionScript. For
> example, if a <script> appears directly in the original HTML document, then
> there is no reasonable JS script that we can say "introduced" the element.
I see that make sense.

And what about having the introductionScript set only if it's created dynamically?
(could be done by dynamically created script).
This would help to compose unique URL through URL chaining (use parent script URL
that already uniquely identifies the parent script, that uses parent script URL, etc.)

There can be entire tree of dynamically created scripts and the URL can indicate what was the parent script (and grouping based on such URL chaining can simplify the UI). 

> However, <script> element code should always have an associated 'element'.
> Could we synthesize names for them by looking at ownerDocument.location.href
> (in a content-distrustful way, of course)?
Yes, that's what I am using now. The element is good for user-cross-link (Script panel -> HTML panel), but it doesn't have to have ID, the xpath is not much unique (e.g. script[2], script[3]),
so not that useful to generate URL (unless I am missing something).

Honza

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

> And what about having the introductionScript set only if it's created
> dynamically?
Btw. this would also help to distinguish between statically and dynamically
created scripElement scripts (which is currently impossible, or am I wrong?)

Honza

Comment 19

[Jim Blandy :jimb]

(In reply to Jan Honza Odvarko from comment #17)
> And what about having the introductionScript set only if it's created
> dynamically?
> (could be done by dynamically created script).
> This would help to compose unique URL through URL chaining (use parent
> script URL
> that already uniquely identifies the parent script, that uses parent script
> URL, etc.)
> 
> There can be entire tree of dynamically created scripts and the URL can
> indicate what was the parent script (and grouping based on such URL chaining
> can simplify the UI). 

Grouping things this way sounds like it could be nice. Filed as bug 983297.

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jim Blandy :jimb from comment #19)
> Grouping things this way sounds like it could be nice. Filed as bug 983297.
Awesome!
Honza