241210 - Newly created Bookmarks have a keyword of "undefined"

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[PikeUK]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040420 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040420 Firefox/0.8.0+

When adding a bookmark the keyword field always contains the word "undefined",
instead of being blank.

Reproducible: Always
Steps to Reproduce:
1. Go to Bookmarks->Add to Bookmarks.
2. Press OK in the resulting dialog box (changing the location first if desired).
3. Right-click the newly created Bookmark and choose Properties.

Actual Results:  
The keyword field contains the word "undefined".

Expected Results:  
The keyword field should be empty.

Regression Window:
OK in 2004-04-09 (Official nightly)
Broken in 2004-04-10 (Official nightly)

Cause:
http://bonsai.mozilla.org/cvsview2.cgi?subdir=mozilla/browser/components/bookmarks/content&command=DIFF_FRAMESET&file=addBookmark2.js&rev1=1.24&rev2=1.25

gKeyword.value = window.arguments[7]; should check the argument actually exists
before assigning it (or alternatively change all callers so it really does exist).

On a closely related note the checkin added gSuggestedKeyword to the file but
never used it.

Comment 1

[PikeUK]

Attachment: Check the keyword value first (and make use of gSuggestedKeyword)

Mike, is this within your review area or should I request review from an FF
peer?

Comment 2

[Mike Connor [:mconnor]]

I can review this, looks perfectly sane on the surface, will give it a more 
formal review tonight (please actually request the review or I'll forget)

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 148210 [details] [diff] [review]
Check the keyword value first (and make use of gSuggestedKeyword)

works great.  r=[e-mail]

Comment 4

[Mike Connor [:mconnor]]

checked in on trunk

Comment 5

[:deanis74]

You're reviewing now Mike?  Does that mean that
http://mozilla.org/projects/firefox/review.html is out of date?

Comment 6

[Mike Connor [:mconnor]]

its out of date in a way, in that ben OK'd me reviewing minor stuff like this, 
but I'm not actually a peer, and I'd rather this fly under radar or I'll just 
get a deluge of requests that I don't have time to keep up with.  (I still have 
a backlog of checkins to do at some point)

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

v.

Comment 8

[PikeUK]

Mike, the Aviary branch needs this patch as well.

Comment 9

[Mike Connor [:mconnor]]

new revision: 1.25.4.2; previous revision: 1.25.4.1

fixed on branch (I thought this would get picked up in the merge to branch, but
I guess not)

Comment 10

[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