625114 - Port |Bug 402147 - tabbrowser event handling cleanup| to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Serge Gautherie (:sgautherie)]

Bug 625056 misses at least "<method name="_getTabForContentWindow">"...

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Just port (the relevant part of) it

With this patch, I get
{
Error: aTab is null
Source File: chrome://navigator/content/tabbrowser.xml
Line: 1251
}
when closing a tab.

It happens at the first line of setTabTitle().

I'm not sure whether I did something wrong in porting that changeset or if this depends on porting yet another bug first...

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> I'm not sure whether I did something wrong in porting that changeset or if this
> depends on porting yet another bug first...

I'm guessing bug 449728 would help...

Comment 3

[[e-mail]]

(In reply to comment #1)
> With this patch, I get
> {
> Error: aTab is null
> Source File: chrome://navigator/content/tabbrowser.xml
> Line: 1251
> }
> when closing a tab.
This is because when we close a tab we don't destroy it immediately, so that we can undo close it very quickly. But this means we can get title change events from hidden browsers, so checking for .top doesn't work.

Comment 4

[[e-mail]]

Comment on attachment 504554 [details] [diff] [review]
(Av1) Just port (the relevant part of) it

>-                    onclosetab="var node = this.parentNode;
>-                                while (node.localName != 'tabbrowser')
>-                                  node = node.parentNode;
>-                                node.removeCurrentTab();">
Doesn't this stop the tabstrip close button from working?

>-      <field name="_keyEventHandler" readonly="true">
>+      <method name="_handleKeyEvent">
Nit: method doesn't belong here. As you're already losing all but one line of history, might as well merge it into handleEvent.

>+            return tab ? tab._tPos : -1;
We don't do tab._tPos

>-            var browsers = this.browsers;
>-            for (var i = 0; i < browsers.length; i++)
>-              if (browsers[i].contentDocument == aDocument)
>-                return browsers[i];
>-            return null;
>+            var tab = this._getTabForContentWindow(aDocument.defaultView);
>+            return tab ? tab.linkedBrowser : null;
This does unnecessary extra work. If you want to share code, make getBrowserIndexForDocument the master method.

>+          for (let i = 0; i < this.browsers.length; i++) {
Nit: should cache this.browsers in a local variable.

>-            var tab = document.getAnonymousElementByAttribute(tabBrowser, "linkedpanel", this.parentNode.id);
Aha, now this is how to get a tab from a browser. Someone wanted a method for this, I think.

Comment 5

[Ian Neal]

Attachment: Update event handling

This patch ports the event handling changes from:

• Bug 402147 - Clean up and refactor some event handling code in tabbrowser.xml
  Plus the error fix from:
• Bug 1077652 - tabbrowser should ignore DOMTitleChanged events from browser that have no tab assigned
  It also includes:
• moving individual handling of each keycode into a catch all
• combining moveTabLeft and moveTabRight into a more general moveTabOver similar to Firefox's code

Other changes from Bug 402147 are either not needed, have already landed or will be parts of other bugs.

Comment 6

[bblack]

works for me with SM2.53

Comment 7

[Ian Neal]

Attachment: Update event handling v1.1

I've decided to keep moveTabLeft and moveTabRight methods for backwards compatibility for extensions, though internally moveTabOver is used.
Made moveTabOver use the same type of parameter as Firefox does, just in case any extensions expect it to be that way.

Comment 8

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9120271 [details] [diff] [review]
Update event handling v1.1

LGTM r/a+

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b1e99d195a47
Port parts of |Bug 402147 - Clean up and refactor some event handling code in tabbrowser.xml| to SeaMonkey. r=frg