952265 - Expose tab 'rename' and 'move' events.

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, enhancement, P2)

Description

[Ryan Stein]

This is a feature request for 'rename' and 'move' events for the SDK Tabs module. I have documented what changes are needed for this here: https://github.com/RyanStein/addon-sdk/commit/85cd301479e79cfb2359b108301c3f14128b8e5b

This is my first time contributing to Mozilla, so please let me know if you have any questions or feedback.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Awesome! The next step is to make a pull request and link it here.

Word of warning: due to holiday closure most Mozillians will be off work starting tomorrow, until Jan 2.

Comment 2

[Ryan Stein]

The pull request has been sent: https://github.com/mozilla/addon-sdk/pull/1331

Happy holidays!

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: GH PR 1331

Pinging Irakli for review for API additions

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8351047 [details] [review]
GH PR 1331

Er, just feedback for API additions, I can do the review

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8351047 [details] [review]
GH PR 1331

Looks good to me, only thing I'm not sure about is "rename" seems like an
awkward event name, specially since we have `tab.title` rather than `tab.name`.
I would vote for `titleChange` event instead.

I'm fine with keeping event name `rename` if native English speaker like yourself (@jsantell) thinks `rename` is more appropriate.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Sorry this has taken so long to get to!

Looks good for the most part, but some changes:

* `rename` event should be `titlechange` (with the constructor handler being `onTitlechange`). `rename` sounds like an active event that was user-initiated, and title change is in line with the title property.

* We can get rid of the documentation in the repository -- they're now independently on MDN (https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/tabs) and once this lands in master, can add the appropriate new events there with notes that they are FF31+

* Would like to see tests for when the `titlechange` event is NOT fired -- there's some logic in there that I'd like to see tested to see when changing data on a tab or scenarios where this event shouldn't be fired actually confirms that the event isn't fired. the `move` event probably doesn't need this as there's less logic on our end for that.

Again, sorry this took so long to get to -- ping me with feedback/review/needinformation on here though to make sure I have it in my queue!

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

Can we leave this to a third party module?

Comment 8

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #7)
> Can we leave this to a third party module?

I think it's absolutely reasonable to have it in core, I'll take over this patch as contributor clearly lost interest in finishing it.

Comment 9

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562