1118194 - Clicking Validate link on Feed Subscriptions dialog does nothing

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

STR:
0. Have some RSS feed(s) set up
1. Open MailNews window
2. Select Subscribe from a feed's context menu
3. Select some feed
4. Click Validate.

The Error Console will then show:

Error: TypeError: this.mMainWin.openContentTab is not a function
Source File: chrome://messenger-newsblog/content/feed-subscriptions.js
Line: 1816

AFAICS there is no function called openContentTab in /suite/ (SM) code. Either it existed at one time, or the /mailnews/ (shared) code always relied on its presence in /mail/ (TB) code.

The referenced file is here:
/mailnews/extensions/newsblog/content/feed-subscriptions.js

Depending on how this needs to be fixed, this bug might have to be moved to MailNews Core.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

BTW FeedMessageHandler.loadWebPage may also call openContentTab, but I couldn't find a caller of it which actually sets the tab or window property on the second argument.

FeedMessageHandler lives here:
/mailnews/extensions/newsblog/content/newsblogOverlay.js

Comment 2

[Philip Chee]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #0)

> STR:
> Error: TypeError: this.mMainWin.openContentTab is not a function
> Source File: chrome://messenger-newsblog/content/feed-subscriptions.js
> Line: 1816

openContentTab is a Thunderbird specific function.
http://mxr.mozilla.org/comm-central/source/mail/base/content/utilityOverlay.js?rev=a005bdfa37f3&mark=236-243#229

Simplest option is to create a stub openContentTab() in our utilityOverlay.js that just redirects to our openUILinkIn(url, where) e.g.

function openContentTab(url, where, handlerRegExp) {
  openUILinkIn(url, where) // handlerRegExp not relevant to SeaMonkey
}

Comment 3

[Philip Chee]

Attachment: Patch v1.0 Proposed fix

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 8544800 [details] [diff] [review]
Patch v1.0 Proposed fix

Yes, that should do; thought the same.

Nits: Would put a semicolon before the comment and maybe change the parameter names to start with "a". Optionally add a comment denoting that this is a TB compat function.

Comment 5

[Philip Chee]

Attachment: Patch v2.0 move function to tabmail.js

Changes in this patch
1. start parameter names with "a".
2. add missing statement terminator semicolon.
3. add a comment explaining that this is a Thunderbird compatibility shim. Some code in shared /mailnews/ expects this function to exist.

Comment 6

[Ian Neal]

Comment on attachment 8547757 [details] [diff] [review]
Patch v2.0 move function to tabmail.js

>+// Thunderbird compatibility function called from e.g. newsblog.
>+function openContentTab(aUrl, aWhere, aHandlerRegExp)
>+{
>+  openUILinkIn(aUrl, aWhere); // aHandlerRegExp is not relevant to SeaMonkey.
>+}

Rather than have two separate comments, just have one covering both points.
r=me with that addressed

Comment 7

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/da03eac9921d

Comment 8

[Ian Neal]

Comment on attachment 8547757 [details] [diff] [review]
Patch v2.0 move function to tabmail.js

[Triage Comment]
a=me for c-a and c-b

Comment 9

[Philip Chee]

Pushed to branches:
http://hg.mozilla.org/releases/comm-aurora/rev/e9001d508454
http://hg.mozilla.org/releases/comm-beta/rev/c35e400193b2

Comment 10

[Philip Chee]

Comment on attachment 8547757 [details] [diff] [review]
Patch v2.0 move function to tabmail.js

[Approval Request Comment]
Regression caused by (bug #): newsblog calling a Thunderbird specific function.
User impact if declined: Unable to verify RSS feeds.
Testing completed (on m-c, etc.): Landed on SeaMonkey 2.35, 2.34, 2.33
Risk to taking this patch (and alternatives if risky): none adds a function that newsblog expects to exist.
String changes made by this patch: none.

Comment 11

[Philip Chee]

http://hg.mozilla.org/releases/comm-release/rev/941c5716f2e1