333052 - places toolbar view should observe changes to titles generated by microsummary service

Status: RESOLVED FIXED

(Firefox Graveyard :: Microsummaries, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

The places toolbar view should observe changes to titles generated by the microsummary service (and any other services that want to set bookmark titles) and update itself when the generated title changes.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: makes toolbar view observe changes to generated title annotation

Here's a patch that makes the places toolbar view observe changes to the "generatedTitle" annotation and update bookmark labels accordingly.

Design issues to consider in review:

1. Is "generatedTitle" the right name for this annotation?

2. I implemented a generic annotation observer that manages annotation-specific
   observers (of which there is currently just one: the generatedTitle
   annotation observer).  The generic observer will reduce code duplication
   if the view ever starts observing another annotation, and it might be useful
   for extensions that want to make the view observe other annotations,
   but perhaps it's overkill.

Comment 2

[Brett Wilson]

Some comments:

I think we should use "bookmarks/generatedTitle" as the annotation name. I'm trying to get everybody to namespace their annotations, so we should set a good example.

I was wondering if we should change the annotation observer API so you can specify which annotations you are interested in. I think this would also be helpful for the annotation service.

Comment 3

[Annie Sullivan]

Comment on attachment 217491 [details] [diff] [review]
patch v1: makes toolbar view observe changes to generated title annotation

r=[e-mail], with the condition that you change generatedTitle to bookmarks/generatedTitle

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: generatedTitle -> bookmarks/generatedTitle

> I think we should use "bookmarks/generatedTitle" as the annotation name. 
> I'm trying to get everybody to namespace their annotations, so we should set 
> a good example.

> r=[e-mail], with the condition that you change generatedTitle
> to bookmarks/generatedTitle

Makes sense.  I've made the change in this patch.


> I was wondering if we should change the annotation observer API so you can
> specify which annotations you are interested in. I think this would also be
> helpful for the annotation service.

Yeah, that may well be a better long-term solution.

Comment 5

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

Comment on attachment 217598 [details] [diff] [review]
patch v2: generatedTitle -> bookmarks/generatedTitle

sr=[e-mail]

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Thanks all.  Fix checked in to trunk and 1.8 branch.