277490 - Retrieve site description from meta tag when adding bookmark

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, enhancement)

Description

[:Gavin Sharp [email: [e-mail]]]

When adding a bookmark, we should see if the site has <meta name="description">
headers and retrieve the data from it to put into the bookmark.

Patch coming up.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v1

I'll wait for more testing/verification before asking for review. This also
changes the addBookmark2 interface to take an object instead of a list of
arguments, to avoid having to remember the order etc, as mentioned at:

http://lxr.mozilla.org/mozilla/source/browser/components/bookmarks/content/addBookmark2.js#56

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v2

Missed a few things in the initial patch.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v3

More changes. These are things that I should be testing *before* posting
patches.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v4 (Checked in)

Add in a check for the http-equiv="description" tag.

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 170777 [details] [diff] [review]
Patch v4 (Checked in)

I honestly won't have time to do this for a while, vlad would be a better
reviewer for bookmarks code anyway.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 170777 [details] [diff] [review]
Patch v4 (Checked in)

r=[e-mail], looks good, thanks for doing the change to the add
bookmark window arguments as well.

Comment 7

[Ian Neal]

Comment on attachment 170777 [details] [diff] [review]
Patch v4 (Checked in)

Checking in base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.381; previous revision: 1.380
done
Checking in components/bookmarks/content/addBookmark2.js;
/cvsroot/mozilla/browser/components/bookmarks/content/addBookmark2.js,v  <-- 
addBookmark2.js
new revision: 1.33; previous revision: 1.32
done
Checking in components/bookmarks/content/addBookmark2.xul;
/cvsroot/mozilla/browser/components/bookmarks/content/addBookmark2.xul,v  <-- 
addBookmark2.xul
new revision: 1.18; previous revision: 1.17
done
Checking in components/bookmarks/content/bookmarks.js;
/cvsroot/mozilla/browser/components/bookmarks/content/bookmarks.js,v  <-- 
bookmarks.js
new revision: 1.92; previous revision: 1.91
done

Comment 8

[Asaf Romano (gone)]

does it also address live-bookmarks?

Comment 9

[Kelley Cook]

(In reply to comment #8)
> does it also address live-bookmarks?

No, but see Bug 256794 for the live bookmark component.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

The patch that was checked in does have an effect on livemarks, but it isn't
exactly optimal. The "Name" for new live bookmarks is currently taken from the
page where it is embedded, and this patch extends that to the description as
well. I'm sure there are cases where the page's description and the feed's
description are not related at all.

As mentioned, bug 256794 is about improving that behavior.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Attachment: Forgot one caller

Comment 12

[Asaf Romano (gone)]

Checking in browser/components/sidebar/src/nsSidebar.js;
/cvsroot/mozilla/browser/components/sidebar/src/nsSidebar.js,v  <--  nsSidebar.js
new revision: 1.9; previous revision: 1.8
done

Comment 13

[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