can't inline rename bookmarks/folders or add bookmark folders



Bookmarks & History
18 years ago
13 years ago


(Reporter: Brent Hendricks, Assigned: bz)



Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(2 attachments)



18 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.14-5.0 i686; en-US; 0.8.1)
BuildID:    2001032303

After inline editing the name of a bookmark or folder and pressing enter, the
new name doesn't stick.  It goes back to the original name. 

Reproducible: Always
Steps to Reproduce:
1. Pull up the bookmarks manager
2. Select a bookmark or folder
3. Click the "Rename..." button
4. Type a new name in the inline box
5. Press Enter

Actual Results:  The bookmark or folder name goes back to its original value

Expected Results:  The new name should replace the old.

Comment 1

18 years ago
Not an issue in 3-23 Mozilla 0.8.1-6 - FIXED
This broke after 0.8.1 branched, apparently.  I'm seeing this too, with
2001-03-23-08 build.

Specifically, when you click on a bookmark to edit it inline, the field comes up
completely empty instead of having the old text in it.  Whatever you type in the
field is not saved.
Ever confirmed: true

Comment 3

18 years ago
Adding keywords
Keywords: regression, ui

Comment 4

18 years ago
sooooo, i see this broken on a 2001032304 Win build but fixed on a 2001032310 mac build
AND on a 2001032311 Linux build.

that tells me based on the reporter's build ID a fix was checked in btw 2001032303 and ...04.
I thought that might be of interest to people.

It probably wouldn't be too hard to check the logs and see who did what during that hour...but
instead I'm just going to mark this fixed and verified.
Last Resolved: 18 years ago
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED

Comment 5

18 years ago
VERIFIED Fixed (with 2001032311 builds)

Comment 6

18 years ago
Is it really fixed? With build 2001032321 under linux the rename of bookmarks
doesn't work. Also if I try to create a new folder I'm unable to give the folder
a name. 
If I right-click a bookmark the menu opens but nothing works.
Still broken for me in a 2001-03-24 morning CVS build on Linux.

Comment 8

18 years ago
I think this is mine.
Resolution: FIXED → ---

Comment 9

18 years ago
Reassigning to me .
Assignee: ben → blakeross

Comment 10

18 years ago
indeed Blake, it's yours..
in xpfe/global/resources/content/treeBindings.xml, line 152 and line 134 you 
changed box.lastChild.value into box.lastChild.label.
However lastChild is a textbox, and so it had to stay .value.
Anyone care to produce a patch?
*** Bug 73369 has been marked as a duplicate of this bug. ***
Attaching patch with Fabian's changes and another change that makes the current
value appear highlighted in the textbox like it used to (so you can edit it or
overwrite it).
Keywords: patch, review

Comment 14

18 years ago
note that the attached patch also fixes bug 73369 which has been marked as a dupe but is actually
a slightly more severe manifestation b/c it affects naming new folders as well.

Comment 15

18 years ago
bz, are you sure the first change from label to value has to be done?
box.firstChild is a <text> widget, and <text> widget's use label, not value.
Or am I missing something?
well....  I haven't dug into the code comletely, so I'm not sure why it works... 
but without that change the beavior is wrong.  With the change, the behavior is 
correct.  Try both.  I'll try to figure it out sometime this weekend.

Comment 17

18 years ago
*** Bug 74099 has been marked as a duplicate of this bug. ***
The xul:text has the following attribute set:                  

Would that affect things in any way?  Note: getAttribute("value") returns the
right thing.  getAttribute("label") returns "", .label returns "undefined"

Comment 19

18 years ago
doh! you're right, my mistake..
*** Bug 74255 has been marked as a duplicate of this bug. ***

Comment 21

18 years ago
With this patch applied I can rename/create folders again, but each time I
create a folder I get this error:

JavaScript error: 
line 7: cell.setMode is not a function

I don't know if it is related, but I thought I would report it anyway.
Well, it's in the same file.. So attaching updated patch that gets rid of that
error too.  :)
Created attachment 29407 [details] [diff] [review]
patch that checks for existence of cell.setMode too

Comment 24

18 years ago
Assignee: blakeross → bzbarsky
Keywords: review, ui → approval

Comment 25

18 years ago
timeless: Stop reassigning my goddamn bugs, this is like the 8th time you've 
done this (and the 8th time I've said to stop).  These bugs are on my list for 
a reason; so I can look over the attached patches and check them in if 
necessary. At the very least, you could have the courtesy to cc me.  I'll 
reassign it to bzbarsky when I'm ready to check it in.
Assignee: bzbarsky → blakeross

Comment 26

18 years ago
*** Bug 74329 has been marked as a duplicate of this bug. ***
*** Bug 74412 has been marked as a duplicate of this bug. ***

Comment 28

18 years ago
changing summary to capture the nature of the dupes, since the belief is that
these bugs are all the same root problem.
Summary: Inline rename of bookmarks doesn't work → can't inline rename bookmarks/folders or add bookmark folders

Comment 29

18 years ago
*** Bug 74116 has been marked as a duplicate of this bug. ***


18 years ago
Blocks: 68496

Comment 30

18 years ago
*** Bug 74255 has been marked as a duplicate of this bug. ***

Comment 31

18 years ago
*** Bug 74859 has been marked as a duplicate of this bug. ***

Comment 32

18 years ago
Cc'ing alec for sr.

Comment 33

18 years ago
Blake, this is reviewed and all. Do you want to check it in or should we just
wait for Ben to check in his fix for this (I can't find the relevant bug right now)
*** Bug 75381 has been marked as a duplicate of this bug. ***

Comment 36

18 years ago
blake: bugs should be assigned to their owners, if you need a way to track bugs 
you need to check in, use something else.
Assignee: blakeross → bzbarsky

Comment 37

18 years ago
fix checked in
Last Resolved: 18 years ago18 years ago
Keywords: approval
Resolution: --- → FIXED

Comment 38

18 years ago
Round and round we go!

timeless, I own this bug because I'm the dumbass that caused it.  I already 
said I'm fully willing to reassign to bz once_I_check_in_his_fix.  I appreciate 
this fix, I really do.  I hesitated to check it in for good reason, because I 
knew Ben was working on an extended fix simultaneously.

If you would like to write and explain to them why bz needed 
to own this bug for those few days, please go right ahead.  Don't forget to 
mention that you went and checked in this patch when even its creator was 
kindly willing to ask whether we should just wait for Ben's first.

That said, I'm reopening this bug and marking it a duplicate of Ben's bug.  Ben 
is almost certainly going to back the fix out anyways.  Boris, I'm sorry you 
had to be caught in the middle of this.  Thanks again for this patch!
Resolution: FIXED → ---

Comment 39

18 years ago

*** This bug has been marked as a duplicate of 68537 ***
Last Resolved: 18 years ago18 years ago
Resolution: --- → DUPLICATE

Comment 40

17 years ago
VERIFIED Dupe since ben's fix for that bug incorporates a fix for this bug.


17 years ago
Depends on: 77125
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.