668258 - Some (all?) content-related APIs sometimes don't work for pages that don't have DOMs

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Drew Willcoxon :adw]

This is the general case of the problem described in bug 667957.  page-mod for one has this problem when contentScriptWhen == "ready", since it listens for DOMContentLoaded in that case.  This mailing list post says it's also a problem for tabs.on("ready"):

http://groups.google.com/group/mozilla-labs-jetpack/t/fe9b3051cf5da6e7

Alex or Irakli, could you take a look at the content-related APIs, file bugs, and make sure this gets fixed?

(Actually, if you inspect a page that's "only" an image file, you'll see it does in fact have a DOM.  I guess that's just an implementation detail within the browser.)

Comment 1

[Alexandre Poirot [:ochameau]]

I already faced such issue while working with Symbiont and added similar code:
https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/content/symbiont.js#L132

But this may not work correctly with "ready" ...
I'll take a look at it.

Comment 2

[Drew Willcoxon :adw]

Thanks Alex.  It looks like there's basically the same code in context-menu, page-mod, and symbiont (and other places?) to handle page loading and contentScriptWhen.  It would be great if they all used symbiont or otherwise shared code.

Comment 3

[Alexandre Poirot [:ochameau]]

In fact these contents have DOM, but a different kind of "document".
For exemple, if you load an image:

You end up having an ImageDocument instead of a HtmlDocument.
The precise issue is that page-mod rely on "document-element-inserted" event,
and this event is not dispatched on such document.
I tried to guess by reading c++ code if we can rely on "content-document-global-created" and then wait for readystate event. But I have absolutely no idea if 
"document-element-inserted" is, by any chance, equivalent to readystate=loading or readystate=interactive ?
("content-document-global-created" is dispatched on any kind of document)

Comment 4

[Alexandre Poirot [:ochameau]]

I filled bug 678465, for the platform bug.

Comment 5

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

(Pushing all open bugs to the --- milestone for the new triage system)

Comment 6

[Gabor Krizsanits (INACTIVE)]

(In reply to Alexandre Poirot (:ochameau) from comment #3)
> In fact these contents have DOM, but a different kind of "document".
> For exemple, if you load an image:
> 
> The precise issue is that page-mod rely on "document-element-inserted" event,
> and this event is not dispatched on such document.
I think it should be for every document, that's what I'm trying to fix.

> I tried to guess by reading c++ code if we can rely on
> "content-document-global-created" and then wait for readystate event. But I
> have absolutely no idea if 
> "document-element-inserted" is, by any chance, equivalent to
> readystate=loading or readystate=interactive ?
> ("content-document-global-created" is dispatched on any kind of document)

The first difference between the two event that you will get a ref to the document in the document-element-inserted, as opposed to the content-document-global-created where you get the window. The later one is also a bit closer to the window creation instead of the document creation, see 549539 comment 33. Meaning the doc might not be set on the window in every case... I'm not sure if that has been changed or not, and I don't really have a use case for it, but makes me a bit worried assuming that you want the document up and running at that point for addons.

Comment 7

[Alexandre Poirot [:ochameau]]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #6)
> The first difference between the two event that you will get a ref to the
> document in the document-element-inserted, as opposed to the
> content-document-global-created where you get the window. The later one is
> also a bit closer to the window creation instead of the document creation,
> see 549539 comment 33. Meaning the doc might not be set on the window in
> every case... I'm not sure if that has been changed or not, and I don't
> really have a use case for it, but makes me a bit worried assuming that you
> want the document up and running at that point for addons.

Oh I was not thinking about using only global-created, but global-create AND waiting for `readystatechange` too. We receive multiple values from this event: loading, interactive and loaded.
I think that `loaded` is equivalent to `load` window event,
`interactive` to `DOMContentLoaded` and I was guessing that `loading` was equivalent or close to document-element-inserted?

Comment 8

[Gabor Krizsanits (INACTIVE)]

> I think that `loaded` is equivalent to `load` window event,
> `interactive` to `DOMContentLoaded` and I was guessing that `loading` was
> equivalent or close to document-element-inserted?

According to my understanding, I'd say the difference is that the loading event does not guarantee that the documentElement is attached to the document. The document-element-inserted does guarantee that. Based on the spec this is the difference (you should ask some DOM expert since I'm clearly not one...). Other than that yes, it is close to it. It is between loading and complete for sure.

Comment 9

[Alexandre Poirot [:ochameau]]

I tried using readystatechange event on top of content-document-global-created but there is many edge cases!
- data URI doesn't dispatch `loading` event. Their readyState is `uinitialized` on global-created and there we only receive `interactive` and `complete` that comes too late
- regular remote url or local files have a different behavior. Their readyState is `loading` during global-created, so that we have to evaluate script immediatly if we want to be effective before page <script> evaluations
- if the page call alert(), we won't receive readyState event before user dismiss the alert dialog. But in any case, we have to execute content script before page script execution, so it is not really a problem. Just something to keep in mind for other kind of bugs!

So It seems that we can evaluate content script on global-created event in most cases [need to be 100% confirmed], but I have no solution for data URIs except using document-element-inserted. For them, global-created fires too soon, document is not ready and refer to the temporary about:blank document. And readystatechange doesn't fire on `loading`.

It may be easier to confirm global-created behavior and fix data URIs instead of adding document-element-inserted for every kind of documents?

Comment 10

[Alexandre Poirot [:ochameau]]

FYI: readyState="loading" is set and fired here:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#2321

Comment 11

[Gabor Krizsanits (INACTIVE)]

Attachment: page-mod test for image and video documents

After patches for bug 697955 and bug 678465 land, these tests should be added to jetpack tests.

Comment 12

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562