Implement onBeforeLinkTraversal in MailNews 3pane window

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
MailNews: General
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 years ago
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).
(Assignee)

Comment 1

7 years ago
Created attachment 510003 [details] [diff] [review]
Patch v1.0 dummy onBeforeLinkTraversal implementation.

Just a SMOP.
Attachment #510003 - Flags: superreview?(neil)
Attachment #510003 - Flags: review?(mnyromyr)

Comment 2

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

[Indentation style doesn't match existing weird style...]
Attachment #510003 - Flags: superreview?(neil) → superreview+

Comment 3

7 years ago
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.
Attachment #510003 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 4

7 years ago
Created attachment 512433 [details] [diff] [review]
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.
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3

Updated

6 years ago
Blocks: 649344
You need to log in before you can comment on or make changes to this bug.