225121 - Uncheck 'Load this bookmark in the sidebar' failed

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Lepev]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.6a) Gecko/20031107 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.6a) Gecko/20031107 Firebird/0.7+

Hi,

I have drop a link on the toolbar, right click on it, rename it and check the
box 'Load this bookmark in the sidebar'. Then I click ok, I right click again on
the bookmark, uncheck the box 'Load this bookmark...', click ok. If I verify,
the check still checked, my last opperation has not been saved.

Config: Windows 2003 server-AMD XP 1800-512Mb

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Jason Barnabe (np)]

Confirming on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a)
Gecko/20031107 Firebird/0.7+

Reduced steps to reproduce:
1. Drag link onto personal toolbar.
2. Open properties for that new personal toolbar entry
3. Check "Load this bookmark in the sidebar"
4. Press OK.
5. Open properties for that new personal toolbar entry
6. Uncheck "Load this bookmark in the sidebar"
7. Press OK.
8. Open properties for that new personal toolbar entry

Expected results:
"Load this bookmark in the sidebar" is unchecked

Actual results:
"Load this bookmark in the sidebar" is unchecked

Every subsequent time you try it, the samr thing happens.

Comment 2

[Jason Barnabe (np)]

Correction: instead of "Personal toolbar" make it "Bookmarks toolbar."

It's also broken in the Bookmarks menu item and in the Manage Bookmarks screen.

Comment 3

[Ali Ebrahim]

Setting to All/All. I'm seeing this on Windows and Linux with 2003-11-22.

Comment 4

[James Ross]

FWIW, I don't see any JavaScript errors when this happens.

Comment 5

[PikeUK]

This happens because in bookmarksProperties.js is the code "if (newValue) { do
some updates }", but when you uncheck the Web Panel box newValue is set to
undefined, so the update code in the if statement will never get executed.

I'm not sure the most appropriate way to fix this but an easy (though not very
elegant) way is to make an exception for the webpanel box by changing line 220
to "if (newValue || gFields[i] == "webpanel") {" and line 221 to "if (newValue)
newValue = RDF.GetLiteral(newValue);".

Comment 6

[PikeUK]

Attachment: Patch with "easy fix" from comment #5

The line numbers I gave earlier were slightly wrong but for what it's worth
here's the patch version, maybe someone can fix this in a nicer way.

Comment 7

[Pierre Chanial]

The fix for this bug is much simpler:

diff -u -r1.13 bookmarksProperties.js
--- bookmarksProperties.js      8 Oct 2003 08:17:22 -0000       1.13
+++ bookmarksProperties.js      6 Dec 2003 23:58:29 -0000
@@ -253,10 +253,10 @@
         newValue = "http://" + newValue;
     }
  
-    if (newValue) {
+    if (newValue)
       newValue = RDF.GetLiteral(newValue);
-      changed |= updateAttribute(gProperties[i], oldValue, newValue);
-    }
+
+    changed |= updateAttribute(gProperties[i], oldValue, newValue);
   }
  
   // Update bookmark schedule if necessary;

marking fixed.

Comment 8

[Simon Paquet [:sipaq]]

verified fixed 2004-01-31 trunk build on W2K

Comment 9

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change