595395 - Add pin events to AllTabs.jsm

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect, P3)

Description

[Ian Gilman [:iangilman]]

In bug 593871 we're listening to tab pin/unpin events ourselves, rather than going through AllTabs. Maybe we should add those events to AllTabs?

Comment 1

[Dão Gottwald [:dao]]

Yes.

Comment 2

[Aza Raskin [:aza]]

It looks like Mardak hasn't made much progress, so I am punting this to the future.

Comment 3

[Raymond Lee [:raymondlee]]

Attachment: v1

Re-factoring the code so no test is included.

Comment 4

[Ed Lee :Mardak]

(In reply to comment #3)
> Re-factoring the code so no test is included.
But the alltabs.jsm is changing functionality.

Comment 5

[Raymond Lee [:raymondlee]]

(In reply to comment #4)
> But the alltabs.jsm is changing functionality.

OK, I will add a test for the patch then. :-)

Comment 6

[Ian Gilman [:iangilman]]

Comment on attachment 488831 [details] [diff] [review]
v1

Looks lovely!

Comment 7

[Ian Gilman [:iangilman]]

Note that this patch will also fix bug 597324.

Comment 8

[Raymond Lee [:raymondlee]]

Attachment: v1.1

With test

Comment 9

[Ian Gilman [:iangilman]]

Comment on attachment 489095 [details] [diff] [review]
v1.1

Seems like this test shouldn't access TabView at all... it should just load the AllTabs module and access it directly. 

For that matter, the test might as well be called browser_tabview_alltabs.js.

Comment 10

[Raymond Lee [:raymondlee]]

Attachment: v1.1

Updated the test per Ian's comment

Comment 11

[Ian Gilman [:iangilman]]

Comment on attachment 489401 [details] [diff] [review]
v1.1

Very nice.

Comment 12

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 13

[Ian Gilman [:iangilman]]

Does it need a try run?

Comment 14

[Raymond Lee [:raymondlee]]

Sent that to try c19e54733ca2, waiting for the result.

Comment 15

[Raymond Lee [:raymondlee]]

Passed try!

Comment 16

[Ian Gilman [:iangilman]]

http://hg.mozilla.org/mozilla-central/rev/91465c3b341b