52536 - Can't put bookmark on fullpage plugin

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P1)

Description

[Jazmin Jonson]

From Bugzilla Helper:
User-Agent: Mozilla/4.51 [en] (X11; U; SunOS 5.8 sun4u)
BuildID:    2000082508

When I try to put a bookmark on a fullpage plugin,
I get the following error:
JavaScript Error:
line 0: window._content.location has no properties

This is probably because the parameters given to
BrowserAddBookmark are do not exist for fullpage plugin.
However both the location and the title are present on the 
page when the fullpage plugin is rendered.

I can set bookmarks on fullpage plugins in Netscape4.x

Reproducible: Always
Steps to Reproduce:
1. Go to any file that invokes a fullpage plugin
2.
3.

Actual Results:  JavaScript Error:
line 0: window._content.location has no properties							

Expected Results:  Some how get the title and the location URL and feed that to
the
BrowserAddBookmark() function. Would be nice if the browser created
a document object but this may be hard to do.

Comment 1

[Blake Ross]

Reassigning 79 Bookmarks bugs to Ben.  I was told this was going to be done 
shortly about two months ago, but it clearly hasn't been.  I think that's long 
enough for all these bugs to remain assigned to nobody.

Feel free to filter all this spam into the trashcan by looking for this string 
in the message body: ducksgoquack

Comment 2

[Marcia Knous [:marcia]]

Netscape Nav triage team: this is a Netscape beta stopper.

Comment 3

[Marcia Knous [:marcia]]

*** Bug 58854 has been marked as a duplicate of this bug. ***

Comment 4

[Viswanath Ramachandran]

moving to mozilla0.9. 

Comment 5

[Paul Chen]

Mass moving most of mozilla0.9 bugs to mozilla0.8.1

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

Bugzilla's Create Attachment is broken, so cut and paste this:

Index: bookmarksOverlay.js
===================================================================
RCS file: 
/cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarksOverlay.js,v
retrieving revision 1.8
diff -u -r1.8 bookmarksOverlay.js
--- bookmarksOverlay.js	2001/03/11 07:28:30	1.8
+++ bookmarksOverlay.js	2001/03/12 09:00:03
@@ -765,11 +765,17 @@
     return krAnonymous;
   },
 
-  addBookmarkToWindow: function (aWindow)
+  addBookmarkToDocShell: function (aDocShell)
   {
-    var url = aWindow.location.href;
-    var title = aWindow.document.title || url;
-    var docCharset = aWindow.document.characterSet;
+    var url = aDocShell.currentURI.spec;
+    var title, docCharset = null;
+    try {
+      title = aDocShell.document.title || url;
+      docCharset = aDocShell.document.characterSet;
+    }
+    catch (e) {
+      title = url;
+    }
     this.addBookmark(url, title, docCharset);
   },
   
Index: bookmarksPanel.js
===================================================================
RCS file: 
/cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarksPanel.js,v
retrieving revision 1.1
diff -u -r1.1 bookmarksPanel.js
--- bookmarksPanel.js	2001/03/11 07:28:31	1.1
+++ bookmarksPanel.js	2001/03/12 09:00:03
@@ -66,11 +66,11 @@
   
   addBookmark: function ()
   {
-    // This looks evil, you'd think we'd want to find the most recent NAVIGATOR
-    // window and add a bookmark to the page loaded in that, but that's not the
-    // case. In mail/news, we want to bookmark the current mail message and in
-    // editor we want to bookmark the current document. 
-    BookmarksUtils.addBookmarkToWindow(top._content);
+    // This is somewhat of a hack, and we'd like to parameterize this so that
+    // eventually we can bookmark mail messages and editor documents.
+    var contentArea = top.document.getElementById('content');
+    if (contentArea)
+      BookmarksUtils.addBookmarkToDocShell(contentArea.webNavigation);
   },
   
   manageBookmarks: function ()
Index: resources/content/navigatorOverlay.xul
===================================================================
RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/navigatorOverlay.xul,v
retrieving revision 1.178
diff -u -r1.178 navigatorOverlay.xul
--- navigatorOverlay.xul	2001/03/10 01:42:41	1.178
+++ navigatorOverlay.xul	2001/03/12 09:00:31
@@ -139,7 +139,8 @@
 	<broadcaster id="Browser:Home"    oncommand="BrowserHome();"/>
 
 	<!-- Bookmarks Menu -->
-	<broadcaster id="Browser:AddBookmark" value="&addCurPageCmd.label;" 	 
oncommand="BookmarksUtils.addBookmarkToWindow(window._content);"/>
+	<broadcaster id="Browser:AddBookmark" value="&addCurPageCmd.label;"
+               
oncommand="BookmarksUtils.addBookmarkToDocShell(document.getElementById('content
').webNavigation);"/>
 	<broadcaster id="Browser:ManageBookmark" value="&manBookmarksCmd.label;" 
oncommand="BrowserEditBookmarks();" />
 
 </broadcasterset>
Index: resources/content/contentAreaContextOverlay.xul
===================================================================
RCS file: 
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaContextOverlay.x
ul,v
retrieving revision 1.8
diff -u -r1.8 contentAreaContextOverlay.xul
--- contentAreaContextOverlay.xul	2001/03/11 07:31:09	1.8
+++ contentAreaContextOverlay.xul	2001/03/12 09:01:02
@@ -109,7 +109,7 @@
       <menuitem id="context-bookmarkpage"
                 value="&bookmarkPageCmd.label;"
                 accesskey="&bookmarkPageCmd.accesskey;"
-                
oncommand="BookmarksUtils.addBookmarkToWindow(window._content);"/>
+                
oncommand="BookmarksUtils.addBookmarkToDocShell(document.getElementById('content
').webNavigation);"/>
       <menuitem id="context-bookmarklink"
                 value="&bookmarkLinkCmd.label;"
                 accesskey="&bookmarkLinkCmd.accesskey;"

Comment 7

[Joe Hewitt (gone)]

I'm not overly attached to what the name of the addBookmarkToDocShell function
should be, as others have debated.  Whatever it's called, the code looks
functional.  sr=hewitt

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

fix checked in

Comment 9

[sairuh (rarely reading bugmail)]

mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]