305216 - Bookmark manager's move button doesnt work anymore

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[elfguy]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050819 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050819 Firefox/1.0+

When clicking on "Move" in the download manager, an XML parsing error occurs:

XML Parsing Error: undefined entry:
Location: chrome://browser/content/bookmarks/addBookmark.xul
Line Numer 93, Column 11

Reproducible: Always

Steps to Reproduce:

Comment 1

[elfguy]

Attachment: error dialog

Comment 2

[Phil Ringnalda (:philor)]

bug 300412 removed addGroup.label and addGroup.accesskey without checking for
other callers, looks like.

Comment 3

[gsshih]

Attachment: removed checkbox from addBookmark.xul

Comment 4

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

Is there code in addBookmark.js that relies on this checkbox? hides it? handles
its event handlers? If so, remove that too otherwise there may be js errors by
removing the XUL element only. If not, let me know and I'll review this patch. 

Comment 5

[gsshih]

Attachment: also removed all dependent code in addBookmark.js

Comment 6

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

Comment on attachment 193186 [details] [diff] [review]
also removed all dependent code in addBookmark.js

r=[e-mail]

Comment 7

[mmc]

Shouldn't this comment be replaced by something else, given that
|window.arguments[5]| is still used a few lines below?

- *   window.arguments[5]: If the mode is "addGroup", this is an array
- *                        of objects with name, URL and charset
- *                        properties, one for each group member.

[...]

+    url = getNormalizedURL(gFld_URL.value);
+    rSource = BMDS.createBookmark(gFld_Name.value, url, null, null,
gBookmarkCharset, false, "");
+    if (window.arguments.length > 4 && window.arguments[4] == "newBookmark") {
+      window.arguments[5].newBookmark = rSource;
+    }    


Also:
@@ -210,7 +186,7 @@
 function onFieldInput()
 {
   const ok = document.documentElement.getButton("accept");
-  ok.disabled = gFld_URL.value == "" && !addingGroup() ||
+  ok.disabled = gFld_URL.value == "" ||
                 gFld_Name.value == "";
 }    
  ^^^^ remove these spaces

Comment 8

[mmc]

Sorry, I didn't mean those spaces, as you didn't touch that code. I meant these
ones...

+      window.arguments[5].newBookmark = rSource;
+    }    
      ^^^^

Comment 9

[Mike Connor [:mconnor]]

checked in branch and trunk