gtkmozembed throws exception on window.blur() which breaks sites

Bug #30349 reported by Ernst Sjöstrand
8
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Fix Released
Medium
Unassigned
Dapper
Invalid
Medium
Unassigned

Bug Description

window.blur() throws an exception which breaks javascript execution on sites that use this. blur() is not implemented for gtkmozembed, but it should not throw an exception.
bsmedberg has made a patch!

Revision history for this message
In , c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount) wrote :

Bug 149307 suggests that gtkmozembed's EmbedWindow class needs to implement nsIEmbeddingSiteWindow2, whose only method is ::Blur.

Revision history for this message
In , Benjamin Smedberg (Mozilla) [:bs] (benjamin-smedbergs) wrote :

Created attachment 210518
Don't throw on unimplemented blur(), rev. 1

Revision history for this message
Ernst Sjöstrand (ernstp) wrote :

window.blur() throws an exception which breaks javascript execution on sites that use this. blur() is not implemented for gtkmozembed, but it should not throw an exception.
bsmedberg has made a patch!

Revision history for this message
Ernst Sjöstrand (ernstp) wrote :
Revision history for this message
In , Jst (jst) wrote :

Comment on attachment 210518
Don't throw on unimplemented blur(), rev. 1

- In nsGlobalWindow::Blur():

   nsresult rv = NS_ERROR_FAILURE;

   nsCOMPtr<nsIDocShellTreeOwner> treeOwner;
   GetTreeOwner(getter_AddRefs(treeOwner));
   nsCOMPtr<nsIEmbeddingSiteWindow2> siteWindow(do_GetInterface(treeOwner));
   if (siteWindow)
     rv = siteWindow->Blur();

   if (NS_SUCCEEDED(rv))
     mDocShell->SetHasFocus(PR_FALSE);

- return rv;
+ // If embedding apps don't implement nsIEmbeddingSiteWindow2, we
+ // shouldn't throw to web content.
+ return NS_OK;

I'd recommend simply just changing what rv is initialized to instead, that way if the blur() function *does* throw an exception it'd make it out to JS rather than getting lost.

r=jst with that change.

Revision history for this message
In , Benjamin Smedberg (Mozilla) [:bs] (benjamin-smedbergs) wrote :

Fixed on trunk. If somebody wants to make gtkmozembed implement nsIEmbeddingSiteWindow2 please file a separate bug.

Revision history for this message
c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount) wrote :

That patch is from usptream bug https://bugzilla.mozilla.org/show_bug.cgi?id=325644 (fixed on trunk but not in ff 1.5.*).

Changed in firefox:
status: Unconfirmed → Rejected
Matt Zimmerman (mdz)
Changed in firefox:
assignee: nobody → ijackson
Ian Jackson (ijackson)
Changed in firefox:
assignee: ijackson → nobody
Changed in firefox:
status: Unknown → Fix Released
Revision history for this message
David Farning (dfarning) wrote :

This issue has been repaired upstream for the 2.X release of firefox.

If it is still an issue for 1.5 shipped with dapper it should be reopened ad ubuntu->dapper->firefox.

Dave

Changed in firefox:
status: Unconfirmed → Fix Released
Changed in firefox:
importance: Unknown → Medium
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.