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

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[musiquegraeme]

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

Comment 1

[Jorg K (CEST = GMT+2)]

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?

Comment 2

[:aceman]

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.

Comment 3

[Jorg K (CEST = GMT+2)]

Clearly a regression, I hope Alice can help us find the culprit which usually gives an indication of the way forward.

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=8898cabfdac3e4744a8aeca8b82bc879f701dbde&tochange=dd4587a78cfd200881956427c8d3fd468aaf9758
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=47e0584afe0ab0b867412189c610b302b6ba0ea7&tochange=47e0584afe0ab0b867412189c610b302b6ba0ea7

Suspect: Bug 1279834

Comment 5

[Jorg K (CEST = GMT+2)]

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]

Comment 6

[Alice0775 White]

(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....

Comment 7

[Jorg K (CEST = GMT+2)]

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?

Comment 8

[:aceman]

On current trunk, clicking the [Learn more] does nothing, not even the error in console.

Comment 9

[ISHIKAWA, Chiaki]

Gosh, this is back.
Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1571645

Comment 10

[:aceman]

Attachment: 1427590.patch add openTrustedLinkIn [landed in comment 12]

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.

Comment 11

[Jorg K (CEST = GMT+2)]

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.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Jorg K (CEST = GMT+2)]

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.

Comment 14

[Jorg K (CEST = GMT+2)]

TB 68.1 ESR:
https://hg.mozilla.org/releases/comm--esr68/rev/e73026ad5377e648a5bd489aededa47cda2c3eb1

Comment 15

[Jorg K (CEST = GMT+2)]

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.

Comment 16

[Jorg K (CEST = GMT+2)]

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.

Comment 17

[Jorg K (CEST = GMT+2)]

Similar to bug 1454041 comment #2.

Comment 18

[:aceman]

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.

Comment 19

[Jorg K (CEST = GMT+2)]

Why don't we just package it?

Comment 20

[:aceman]

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.

Comment 21

[:aceman]

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

Comment 22

[:aceman]

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).

Comment 23

[:aceman]

(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.

Comment 24

[:aceman]

Attachment: 1427590-2.patch package devtools-loader.js

Description of the patch in previous comments.

Comment 25

[Jorg K (CEST = GMT+2)]

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.

Comment 26

[Jorg K (CEST = GMT+2)]

Comment on attachment 9091245 [details] [diff] [review]
1427590-2.patch package devtools-loader.js

I suggested to package it ;-)

Comment 27

[Pulsebot]

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

Comment 28

[Jorg K (CEST = GMT+2)]

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

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

Comment 29

[:aceman]

Also for 68 please.

Comment 30

[Jorg K (CEST = GMT+2)]

Sure, the flag is already set.

Comment 31

[Jorg K (CEST = GMT+2)]

Finally works, yay!!

Comment 32

[Paul Morris [:pmorris]]

(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.

Comment 33

[Jorg K (CEST = GMT+2)]

TB 68.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/9818787d167f997f20f636e11829eeae61cd4a11
Packaging follow-up.