JavaScript error: .../nsSuiteGlue.js, line 291: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIWebProgress.DOMWindow]

RESOLVED FIXED in seamonkey2.38

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mmokrejs, Assigned: philip.chee)

Tracking

SeaMonkey 2.33 Branch
seamonkey2.38
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.38 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0 SeaMonkey/2.33.1
Build ID: 20150425013755

Steps to reproduce:

Not sure if this was triggered by the bugzilla.mozilla.org login process or due to the about:plugins page to be loaded.


Actual results:

--DOMWINDOW == 39 (0x7f25a609c500) [pid = 20201] [serial = 60] [outer = (nil)] [url = https://login.persona.org/communication_iframe]
[20201] WARNING: NS_ENSURE_TRUE(txToRemove) failed: file /var/tmp/portage/www-client/seamonkey-2.33.1/work/comm-release/mozilla/docshell/shistory/src/nsSHistory.cpp, line 1339
JavaScript error: jar:file:///usr/lib64/seamonkey/omni.ja!/components/nsSuiteGlue.js, line 291: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIWebProgress.DOMWindow]
Document about:plugins loaded successfully
> JavaScript error: jar:file:///usr/lib64/seamonkey/omni.ja!/components/nsSuiteGlue.js,
> line 291: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIWebProgress.DOMWindow]

I've seen this error too. CC Neil
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(neil)
OS: Unspecified → All
Hardware: Unspecified → All
Summary: JavaScript error: jar:file:///usr/lib64/seamonkey/omni.ja!/components/nsSuiteGlue.js, line 291: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIWebProgress.DOMWindow] → JavaScript error: .../nsSuiteGlue.js, line 291: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIWebProgress.DOMWindow]
Ugh, about:plugins is HTML, and therefore can't use our chrome entities to synchronise with the UI's direction. Ehsan therefore added an XHTML frame to do this, but he removes the frame while it's still loading, so by the time we get told about it its window no longer exists, thus the error message.

What might work is to skip documents that aren't loading (since we get multiple notifications and we only really care for the first one anyway). I haven't checked to see whether password manager still works if we do this though.
Flags: needinfo?(neil)
> +    if (aStateFlags & Components.interfaces.nsIWebProgressListener.STATE_STOP &&
> +        aStateFlags & Components.interfaces.nsIWebProgressListener.STATE_IS_DOCUMENT)
When I load about:plugins aStateFlags is 0x20010 which corresponds to STATE_STOP + STATE_IS_DOCUMENT

Also ports TOOLKIT patch from Bug 762593 - Add logic for validating if password input fields are insecure.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8598148 - Flags: review?(neil)
Comment on attachment 8598148 [details] [diff] [review]
Patch v1.0 Skip documents that aren't loading

>+    if (aStateFlags & Components.interfaces.nsIWebProgressListener.STATE_STOP &&
>+        aStateFlags & Components.interfaces.nsIWebProgressListener.STATE_IS_DOCUMENT)
>+      return;
Actually there is a suitably named property on the web progress object...

>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", function(event) {
>+      InsecurePasswordUtils.checkForInsecurePasswords(event.target);
>+    });
[Bah, this one uses event.target instead of event?]
Please make a global function, it doesn't need to be in this scope.

What's the UI for this?
Depends on: 1160222
>>+        aStateFlags & Components.interfaces.nsIWebProgressListener.STATE_IS_DOCUMENT)
>>+      return;
> Actually there is a suitably named property on the web progress object...
Oops. Fixed.

>>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", function(event) {
>>+      InsecurePasswordUtils.checkForInsecurePasswords(event.target);
>>+    });
> [Bah, this one uses event.target instead of event?]
> Please make a global function, it doesn't need to be in this scope.
> 
> What's the UI for this?
Bug 947471 was filed to provide a proper UI.

Currently the UI is here:
https://hg.mozilla.org/mozilla-central/rev/a46b44af697f#l2.51
And here:
https://hg.mozilla.org/mozilla-central/rev/2b03e95df6d3#l8.2
Attachment #8598148 - Attachment is obsolete: true
Attachment #8598148 - Flags: review?(neil)
Attachment #8600602 - Flags: review?(neil)
Comment on attachment 8600602 [details] [diff] [review]
Patch v2.0 Check for isLoadingDocument

>   onStateChange: function(aWebProgress, aRequest, aStateFlags, aStatus) {
>+    if (!aWebProgress.isLoadingDocument)
>+      return;
Nit: blank line

>     aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onFormPassword, true);
>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", checkForInsecurePasswords, true);
>     aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>     aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
Nit: onFormPassword and onUsernameInput are linked; keep those lines together.
Attachment #8600602 - Flags: review?(neil) → review+
Updated patch also includes port of Bug 1149975 (Part 1 of 2 - Handle visibility of the login fill doorhanger anchor).

I put the pageshow event handler in navigator.js because in nsSuiteGlue.js I kept getting "event.target is undefined" errors.

I also put the message listener in navigator:
> +  window.messageManager
> +        .addMessageListener("RemoteLogins:fillForm", (message) => {
> +    LoginManagerContent.receiveMessage(message, content);
> +  });
Attachment #8600602 - Attachment is obsolete: true
Attachment #8607201 - Flags: review?(neil)
Also, I asked a question in 1149975 Comment 40
https://bugzilla.mozilla.org/show_bug.cgi?id=1149975#c40
(In reply to Philip Chee from comment #7)
> Updated patch also includes port of Bug 1149975 (Part 1 of 2 - Handle
> visibility of the login fill doorhanger anchor).
> 
> I put the pageshow event handler in navigator.js because in nsSuiteGlue.js I
> kept getting "event.target is undefined" errors.
> 
> I also put the message listener in navigator:
> > +  window.messageManager
> > +        .addMessageListener("RemoteLogins:fillForm", (message) => {
> > +    LoginManagerContent.receiveMessage(message, content);
> > +  });

I don't think navigator is the right place for this because the login manager is sending the message to the content script. (And I guess the pageshow event handler might as well go there too.)
Comment on attachment 8607201 [details] [diff] [review]
Patch v3.0 include port of Bug 1149975

>+function onDOMFormHasPassword(event) {
>+  var content = event.target.ownerDocument.defaultView;
This is not quite right, they want event.target.ownerDocument.defaultView.top

>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onDOMFormHasPassword, true);
>     aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>     aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", checkForInsecurePasswords, true);
Now that you need a separate function for both login manager and insecure password utils, might as well combine the two.
> I don't think navigator is the right place for this because the login 
> manager is sending the message to the content script. (And I guess the 
> pageshow event handler might as well go there too.)
In this patch I've switched to a content script containing the message listener and the pageshow event handler.

>>+function onDOMFormHasPassword(event) {
>>+  var content = event.target.ownerDocument.defaultView;
> This is not quite right, they want event.target.ownerDocument.defaultView.top
Fixed.

>>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onDOMFormHasPassword, true);
>>     aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>>     aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
>>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", checkForInsecurePasswords, true);
> Now that you need a separate function for both login manager and insecure password utils, might as well combine the two.
Fixed.
Attachment #8607201 - Attachment is obsolete: true
Attachment #8607201 - Flags: review?(neil)
Attachment #8610865 - Flags: review?(neil)
Comment on attachment 8610865 [details] [diff] [review]
Patch v5.0 use content frame script

>+  var mm = window.getGroupMessageManager("browsers");
>+  mm.loadFrameScript("chrome://navigator/content/content.js", true);
I think we should start by loading the frame script into all content frames using the global message manager, so that it covers sidebars and RSS feeds and such.

>+function onDOMFormHasPassword(event) {
>+  var content = event.target.ownerDocument.defaultView.top;
>+  LoginManagerContent.onDOMFormHasPassword(event, content);
>+  InsecurePasswordUtils.checkForInsecurePasswords(event.target);
>+}
Doesn't this belong in the content script now?

>   onStateChange: function(aWebProgress, aRequest, aStateFlags, aStatus) {
>-    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onFormPassword, true);
>+    if (!aWebProgress.isLoadingDocument)
>+      return;
>+
>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onDOMFormHasPassword, true);
>     aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>     aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
>   },
[Probably all of this does too.]
Blocks: 1167993
Blocks: 1169709
> I think we should start by loading the frame script into all content 
> frames using the global message manager, so that it covers sidebars and 
> RSS feeds and such.
Done.

>>+function onDOMFormHasPassword(event) {
> Doesn't this belong in the content script now?
Moved to content.js

>>   onStateChange: function(aWebProgress, aRequest, aStateFlags, aStatus) {
>>-    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onFormPassword, true);
>>+    if (!aWebProgress.isLoadingDocument)
>>+      return;
>>+
>>+    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onDOMFormHasPassword, true);
>>     aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>>     aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
>>   },
> [Probably all of this does too.]
Moved to content.js
Attachment #8610865 - Attachment is obsolete: true
Attachment #8610865 - Flags: review?(neil)
Attachment #8614277 - Flags: review?(neil)
Duplicate of this bug: 1170210
Comment on attachment 8614277 [details] [diff] [review]
Patch v6.0 moved almost everything to content.js

A password got filled in so I assume that means it's working.

>   onStateChange: function(aWebProgress, aRequest, aStateFlags, aStatus) {
>-    aWebProgress.DOMWindow.addEventListener("DOMFormHasPassword", onFormPassword, true);
>-    aWebProgress.DOMWindow.addEventListener("DOMAutoComplete", onUsernameInput, true);
>-    aWebProgress.DOMWindow.addEventListener("change", onUsernameInput, true);
>+    if (!aWebProgress.isLoadingDocument)
>+      return;
>   },
This isn't doing anything any more so remove it and remove NOTIFY_STATE_DOCUMENT from _init(). r=me with that fixed.
Attachment #8614277 - Flags: review?(neil) → review+
Once new patch is there, a=me for CLOSED TREE
http://hg.mozilla.org/comm-central/rev/33c0832f86e0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.38
See Also: → 1208971
You need to log in before you can comment on or make changes to this bug.