631770 - Implement onBeforeLinkTraversal in MailNews 3pane window

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Philip Chee]

Bug 610736 implemented a dummy onBeforeLinkTraversal to keep the backend happy but we forgot that MailNews also has a tabbrowser. When testing the CTRL-F4 patch I kept getting this error:

Error: 'JavaScript component does not have a method named:
"onBeforeLinkTraversal"' when calling method: [nsIXULBrowserWindow::onBeforeLinkTraversal] =
NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED

Jens suggested:

> Just a guess: mailWindow.js (assignment on line 129, impl. starting on line 345).

Comment 1

[Philip Chee]

Attachment: Patch v1.0 dummy onBeforeLinkTraversal implementation.

Just a SMOP.

Comment 2

[[e-mail]]

Comment on attachment 510003 [details] [diff] [review]
Patch v1.0 dummy onBeforeLinkTraversal implementation.

[Indentation style doesn't match existing weird style...]

Comment 3

[Karsten Düsterloh]

Comment on attachment 510003 [details] [diff] [review]
Patch v1.0 dummy onBeforeLinkTraversal implementation.

First of all: I wasn't able to reproduce the problem under Linux at all, hence I just do a formal text-based review. While I trust you it exists, I'd be glad to see some str… :-) 

(In reply to comment #2)
> [Indentation style doesn't match existing weird style...]

Well, it's called "GNU style".
Okay, agreed, it *is* weird. :-D
Nevertheless, we should not add more weird looking styled stuff, hence …

>     },
>+  // Called before links are navigated to to allow us to retarget them if needed.
>+  onBeforeLinkTraversal: function(originalTarget, linkURI, linkNode, isAppTab)
>+  {
>+    return originalTarget;
>+  },
>   QueryInterface : function(iid)
>     {

… add a blank line before and after your new block and keep indention as you have it.
Also, please use the 'a' prefix for function parameters.

r=me with that.

Comment 4

[Philip Chee]

Attachment: Patch as checked in.

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/21879c945223

> First of all: I wasn't able to reproduce the problem under Linux at all, hence
> I just do a formal text-based review. While I trust you it exists, I'd be glad
> to see some str… :-) 

Open a few tabs, tab around, close a few tabs (CTRL-F4).

> … add a blank line before and after your new block and keep indention as you
> have it.
Fixed.

> Also, please use the 'a' prefix for function parameters.
Fixed.

> r=me with that.