Closed Bug 1427590 Opened 2 years ago Closed Last month

TypeError: this.chromeUtilsWindow.openUILinkIn is not a function hudservice.js:425:5 - when clicking [Learn More] in error console

Categories

(Thunderbird :: General, defect)

52 Branch
defect
Not set

Tracking

(thunderbird_esr6869+ fixed, thunderbird70 fixed, thunderbird71 fixed)

VERIFIED FIXED
Thunderbird 70.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird70 --- fixed
thunderbird71 --- fixed

People

(Reporter: musiquegraeme, Assigned: aceman)

References

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171206182557

Steps to reproduce:

I click on the [Learn More] in the error console


Actual results:

I get the error message 
TypeError: this.chromeUtilsWindow.openUILinkIn is not a function[Learn More]


Expected results:

The web page should have opened to give me more information on this item
Right, this produces this entry:
TypeError: this.chromeUtilsWindow.openUILinkIn is not a function[Learn More]  hudservice.js:425:5

That function is defined in
https://searchfox.org/mozilla-central/rev/51cd1093c5c2113e7b041e8cc5c9bf2186abda13/browser/base/content/utilityOverlay.js#191
but I guess we don't have it in TB.

Strangely enough this code wasn't changed recently and it was working in TB 52.

Aceman, any clue?
Flags: needinfo?(acelists)
What is 'this code'? Isn't the whole browser console in m-c? They may have changed something so that openUILinkIn() is now used and we do not have that.
Flags: needinfo?(acelists)
Clearly a regression, I hope Alice can help us find the culprit which usually gives an indication of the way forward.
Flags: needinfo?(alice0775)
Keywords: regression
Thanks, so a very old regression from 2016-11-13. That can't be right since it's working in TB 52.x.

Yes, after bug 1279834 was landed, a few things didn't work related to the hudservice, but that was fixed by packaging that service in bug 1318737.

Can you please check whether "[Learn More]" links started working again on 2017-01-17 and then use this as the lower boundary for the regression.

To get a "[Learn More]", open the Error Console, make sure the JS prompt is enabled on the last line (https://developer.mozilla.org/en-US/docs/Tools/Browser_Console, I think: The Browser Console command line is disabled by default. To enable it set the devtools.chrome.enabled preference to true in about:config, or set the "Enable chrome debugging" option in the developer tool settings.).
Then type invalid JS, like jkjk. You get:
  ReferenceError: jkjk is not defined [Learn More]
Flags: needinfo?(alice0775)
(In reply to Jorg K (GMT+1) from comment #5)
> Thanks, so a very old regression from 2016-11-13. That can't be right since
> it's working in TB 52.x.
> 
> Yes, after bug 1279834 was landed, a few things didn't work related to the
> hudservice, but that was fixed by packaging that service in bug 1318737.
> 
> Can you please check whether "[Learn More]" links started working again on
> 2017-01-17 and then use this as the lower boundary for the regression.
> 


...
I can also reproduce the problem even on Daily53.0a1(2017-01-17) as well as TB release 52.5.2....
Flags: needinfo?(alice0775)
I'm sorry, I tried in 52.5.2 and I got that dialogue to choose an application for opening the chrome link. So I assumed it was working and did go any further. Once I chose Firefox, I got the error reported here.

So you're right, it was caused by bug 1279834 and has been broken ever since. Thanks again for finding the regression.

Aceman, any spare cycles to dig through this?
Blocks: 1279834
Component: Untriaged → General
Flags: needinfo?(acelists)
Summary: chromeUtilsWindow.openUILinkLn is not a function → TypeError: this.chromeUtilsWindow.openUILinkIn is not a function hudservice.js:425:5 - when clicking [Learn More] in error console
Version: 57 Branch → 52 Branch
See Also: → 1454041
On current trunk, clicking the [Learn more] does nothing, not even the error in console.

Long story:
The click goes through
https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/devtools/client/webconsole/webconsole.js#155-163
then
https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/devtools/client/shared/link.js#37-43
which calls _getTopWindow:
https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/devtools/client/shared/link.js#14

This one checks for openTrustedLinkIn() function which we don't have so it returns null and then openDocLink cleanly exists without doing anything.

Notice openDocLink does not actually need openTrustedLinkIn()...
Anyway, let's define the function so that we do not hit similar surprises in the future when something actually uses it.

We call it from c-c at https://searchfox.org/comm-central/rev/fcf15ecffc1bbd7670336702cd370f1625706a04/mail/components/customizableui/CustomizeMode.jsm#231. I wonder if that worked or works now.

Assignee: nobody → acelists
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(acelists)
Attachment #9089388 - Flags: review?(jorgk)
Comment on attachment 9089388 [details] [diff] [review]
1427590.patch add openTrustedLinkIn [landed in comment 12]

Wow, I've been waiting for this for over 1.5 years now. Nice research and a fine copy/paste job ;-)

Not that the [Learn More] links are totally useful, but clicking them and spamming the console with even more errors is far worse.

There won't be another TB 69 beta, so this is going out in TB 70 beta next week.
Attachment #9089388 - Flags: review?(jorgk)
Attachment #9089388 - Flags: review+
Attachment #9089388 - Flags: approval-comm-esr68+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/89a77f62b2dd
define openTrustedLinkIn() in contentAreaClick.js as it is used in the Error console to open [Learn more] links. r=jorgk

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED

Paul, we implemented openTrustedLinkIn() in this bug which was missing. As Aceman pointed out in comment #10, this is used here:
https://searchfox.org/comm-central/search?q=openTrustedLinkIn&case=true&regexp=false&path=mail

Could you please check which consequences that has for mail/components/customizableui/CustomizeMode.jsm and file a follow-up bug if more needs to be done.

Flags: needinfo?(paul)
Target Milestone: --- → Thunderbird 70.0

Aceman, that is working fine on trunk, but on TB 68.1 nothing happens (EDIT) when you click a [Learn More] link. Can you get a build from here and look at it again please:
https://treeherder.mozilla.org/#/jobs?repo=comm-esr68&revision=eee024f0004838f21b0fbee315d56375a0607875

You can use the "T/D unpack omni.ja " method to mess around with it.

Flags: needinfo?(acelists)

Damn, this doesn't work in TB 70 beta from automation either, but it worked in a local build of TB 70 when I tested it.

We've seen cases like this before where things in devtools behave differently when run from a local build.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

I determined the file https://searchfox.org/comm-central/source/mail/components/devtools/devtools-loader.js is missing from the omni.ja file of a packaged TB distribution (official releases from server).
When I added the file, the click started working.

The file was added in bug 1279834. It exists when building locally, but isn't included in the packaged application version. I also tested 'mach package' locally which resulted in the same problem.
There are a few other files similarly missing in the omni.ja.

Flags: needinfo?(acelists)

Why don't we just package it?

Flags: needinfo?(philipp)

I'm sure it is intended to be packaged/shipped, it is in moz.build in the EXTRA_COMPONENTS variable.
It is built and lands in the /components folder when built locally, but then for some reason isn't included in the package (omni.ja). 50 other files in EXTRA_COMPONENTS are packaged. That is the mystery here.

Comparing objdir/dist/bin/components with the omni.ja/components folders, there is also odnoklassniki.js file missing in omni.ja. That is for the chat protocol. Needs decision from the chat peers whether that one also needs to be added (is intended to be used in official build).

Flags: needinfo?(clokep)

(In reply to :aceman from comment #21)

I think I found it:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=45107b46a4ee50a81714cb80b0ce603b49869c7c

Bingo, the package from that try build (e.g. https://queue.taskcluster.net/v1/task/bZsAS5wtRhmn61LXDh19Ww/runs/0/artifacts/public/build/target.tar.bz2) does containt the devtools-loader.js now (and the components.manifest file contains the lines from the devtools-loader.manifest), compared to the package from the previous push.

I also downloaded the old and new package (with patch) and verified only the new one opens the [Learn more] link in the current browser.

Description of the patch in previous comments.

Attachment #9091245 - Flags: review?(jorgk)

This is getting confusing. The only NI here should be for Paul in comment #13. Please file a new bug for the chat issue you discovered.

Flags: needinfo?(philipp)
Flags: needinfo?(clokep)
Comment on attachment 9091245 [details] [diff] [review]
1427590-2.patch package devtools-loader.js

I suggested to package it ;-)
Attachment #9091245 - Flags: review?(jorgk)
Attachment #9091245 - Flags: review+
Attachment #9091245 - Flags: approval-comm-esr68+
Attachment #9091245 - Flags: approval-comm-beta+
Keywords: checkin-needed
Attachment #9089388 - Attachment filename: 1427590.patch → 1427590.patch add openTrustedLinkIn [landed in comment 12]]
Attachment #9089388 - Attachment description: 1427590.patch → 1427590.patch add openTrustedLinkIn [landed in comment 12]
Attachment #9089388 - Attachment filename: 1427590.patch add openTrustedLinkIn [landed in comment 12]] → 1427590.patch
Attachment #9091245 - Attachment filename: 1427590-2.patch → 1427590-2.patch package devtools-loader.js
Attachment #9091245 - Attachment description: 1427590-2.patch → 1427590-2.patch package devtools-loader.js
Attachment #9091245 - Attachment filename: 1427590-2.patch package devtools-loader.js → 1427590-2.patch

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5e1353ce04ca
Package missing components/devtools-loader.*. r=jorgk

Status: REOPENED → RESOLVED
Closed: 2 months agoLast month
Keywords: checkin-needed
Resolution: --- → FIXED

TB 70 beta 1:
https://hg.mozilla.org/releases/comm-beta/rev/e8ecfa4e5543844423964cf760dcedb616e6b6e0

The packaging part landed on TB 71, hence it needed backport.

Also for 68 please.

Sure, the flag is already set.

Finally works, yay!!

Status: RESOLVED → VERIFIED

(In reply to Jorg K (GMT+2) from comment #13)

Could you please check which consequences that has for mail/components/customizableui/CustomizeMode.jsm and file a follow-up bug if more needs to be done.

So the code where this is used in CustomizeMode.jsm is not something we are using right now, so there's nothing else to do at this point. If someday we start using this code for customizing toolbars, etc. then it could come into play, need a closer look, etc.

Flags: needinfo?(paul)
You need to log in before you can comment on or make changes to this bug.