1026543 - expose the document.readyState property on tab instances (e10s)

Status: RESOLVED FIXED

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

Description

[Tomislav Jovanovic :zombie]

as described in bug 1017751, tab.onActivate event can happen before the onReady event, before any of the tab's properties (title, url, document) are useful.

with e10s this is all but guaranteed to happen, so Irakli thinks we should expose a property to check what state the tab's document is in, to know if waiting for the 'ready' event is needed or not..

i think we should go with the standard values: uninitialized, loading, interactive and complete, as returned by the document.readyState.

Comment 1

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1517

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8441438 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1517

I'm a little busy, and it's probably better that Irakli review this one.

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

Irakli I can do the review if you just want to f+

Comment 4

[Tomislav Jovanovic :zombie]

Comment on attachment 8441438 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1517

hey Erik, can you please take this review?

this was Irakli's idea originally [1], so i'm pretty sure he will give f+

https://etherpad.mozilla.org/e10s-jetpack

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8441438 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1517

We should created a `tabs.utils` method for getting a tab's document.  Include this and a test for it and the rest looks good.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8441438 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1517

only mentioned a couple of nits in the pull request, I won't block on them.

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/d847667796ca7d3cf2eaf138b7eb9b855e958d33
bug 1026543 - expose tab.readyState

https://github.com/mozilla/addon-sdk/commit/67a98494001c32a4ce74c757b9568ea1c7d03640
Merge pull request #1517 from zombie/1026543-tab-readyState

bug 1026543 - expose tab.readyState property, r=@erikvold

Comment 8

[Will Bamberg [:wbamberg]]

I've updated the page:
https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/tabs
https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/tabs$compare?locale=en-US&to=692175&from=679693

However, I'm not 100% sure of the reasoning around activate. Is the issue that if a new tab is opened, then its activate event may be automatically fired before the tab is loaded, just because of the way tab construction and initialization is implemented?

I can reproduce this problem with an add-on like this:

***
var tabs = require("sdk/tabs");

// Listen for "open"
tabs.on("open", function(tab) {

  // Listen for "activate"
  tab.on("activate", function(tab) {
    logTabState(tab, "activate")
  });

  // Listen for "load"
  tab.on("load", function(tab) {
    logTabState(tab, "load")
  });

  // Listen for "ready"
  tab.on("ready", function(tab) {
    logTabState(tab, "ready")
  });

});

function logTabState(tab, event) {
  console.log(event +
              ": readystate=" + tab.readyState +
              ": url=" + tab.url);
}

tabs.open("https://mozilla.org/");
***
> console.log: tabs: activate: readystate=loading: url=about:blank
> console.log: tabs: ready: readystate=interactive: url=https://www.mozilla.org/en-US/
> console.log: tabs: load: readystate=complete: url=https://www.mozilla.org/en-US/
***

Is this what you meant?