Opening magnet links no longer works

VERIFIED FIXED in Firefox 63

Status

()

P1
normal
VERIFIED FIXED
5 months ago
3 months ago

People

(Reporter: marco, Assigned: kmag)

Tracking

({regression})

Trunk
Firefox 65
regression
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 unaffected, firefox63+ fixed, firefox64+ verified, firefox65 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
Similarly to bug 1498932, caused by either bug 1484496 or bug 1484373.

In this case, I can no longer press on "Open Link", the option is greyed out.

I see "TypeError: handler.equals is not a function" in the Browser Console.

I can only reproduce this in my profile, not in a clean one.
Flags: needinfo?(kmaglione+bmo)
(Reporter)

Updated

5 months ago
See Also: → bug 1498932
(Reporter)

Comment 1

5 months ago
Requesting tracking as this is a recent regression, with a clear regression range, in an user-visible feature.
tracking-firefox63: --- → ?
tracking-firefox64: --- → ?
tracking-firefox63: ? → +
tracking-firefox64: ? → +

Comment 2

5 months ago
If this is the offending code, the issue might manifest itself only Linux:

https://searchfox.org/mozilla-central/source/toolkit/mozapps/handling/content/dialog.js#167
Priority: -- → P1
(Assignee)

Comment 3

5 months ago
Since gioApps here is a plain nsIArray, it doesn't have any intrinsic type
information, and we only get the correct interfaces if they've already been
queried. Adding an interface parameter to the enumerate() call fixes the
issue.

This code is apparently untested, or incompletely tested.

Comment 4

5 months ago
Yes, we have little automated testing of this code.

Can the changes in bug 1484496 or bug 1484373 affect other uses of enumerate() without type information?

https://searchfox.org/mozilla-central/search?q=.enumerate()&case=false&regexp=false&path=*.js
Assuming assignee based on patch.
Assignee: nobody → kmaglione+bmo

Comment 7

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/10476448b17b
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
I'd like QA to verify this patch so as that we can evaluate it for uplift.

Kris, could you request uplift for beta and release channels? This seems like a good candidate for a dot-release ride-along. Thanks
Flags: qe-verify+
(Assignee)

Comment 9

5 months ago
Comment on attachment 9019750 [details]
Bug 1498934: Fix handler app enumeration issue. r=Gijs

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1484496

User impact if declined: This issue prevents some users from opening certain kinds of links in external apps.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment 0. Original reporter should try to verify, since it's not easily reproducible.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It's a trivial change to make sure we query the correct interface when iterating over nsIArray elements.

String changes made/needed: None.
Flags: needinfo?(kmaglione+bmo)
Attachment #9019750 - Flags: approval-mozilla-release?
Attachment #9019750 - Flags: approval-mozilla-beta?
(Reporter)

Comment 10

5 months ago
Verified fixed.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 11

5 months ago
Note, this probably also fixes bug 1498932, which is easily reproducible in a clean profile.

Comment 12

5 months ago
Hi, I tried reproducing this issue but without any luck, the error I'm getting in Browser Console is "TypeError: docShell.failedChannel is null" and not "TypeError: handler.equals is not a function" and I was always able to open the magnet link in a new tab or a new window without any problems except that instead of the Undefined options I'm just getting the:
 
"The address wasn’t understood

Firefox doesn’t know how to open this address, because one of the following protocols (magnet) isn’t associated with any program or is not allowed in this context."

Based on Comment 10 I will mark this issue accordingly.
status-firefox65: fixed → verified

Comment 13

5 months ago
Kris, can the changes in bug 1484496 or bug 1484373 affect other uses of enumerate() without type information, or are we good here?

https://searchfox.org/mozilla-central/search?q=.enumerate()&case=false&regexp=false&path=*.js
Flags: needinfo?(kmaglione+bmo)
Comment on attachment 9019750 [details]
Bug 1498934: Fix handler app enumeration issue. r=Gijs

[Triage Comment]
Fixes issues with users being unable to open some links with external handler apps. Approved for 64.0b6.
Attachment #9019750 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 15

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/c96f6ed751bf
status-firefox64: affected → fixed

Comment 16

4 months ago
Hi Marco, can you please verify the beta version of Firefox as well for this issue, here is our latest beta: https://www.mozilla.org/en-US/firefox/channel/desktop/
Flags: needinfo?(mcastelluccio)
(Reporter)

Updated

4 months ago
status-firefox64: fixed → verified
Flags: needinfo?(mcastelluccio)
Comment on attachment 9019750 [details]
Bug 1498934: Fix handler app enumeration issue. r=Gijs

Fixes issues with users being unable to open some links with external handler apps. One liner, uplift verified on beta, approved for 63.0.3.
Attachment #9019750 - Flags: approval-mozilla-release? → approval-mozilla-release+

Comment 18

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/bb7395925fab
status-firefox63: affected → fixed

Comment 19

4 months ago
Hi Marco, Thanks for all the help, If it's not too much trouble can you please recheck this issue on the latest release version of Firefox? you have the build here: http://archive.mozilla.org/pub/firefox/candidates/63.0.3-candidates/  it will be much appreciated :)
Flags: needinfo?(mcastelluccio)
(Reporter)

Comment 20

3 months ago
Totally forgot about this, luckily it appears to be working :)

The question to Kris still stands, perhaps it would be easier to email him directly or ping him on IRC.
Flags: needinfo?(mcastelluccio)
(Assignee)

Comment 21

3 months ago
(In reply to :Paolo Amadini from comment #13)
> Kris, can the changes in bug 1484496 or bug 1484373 affect other uses of
> enumerate() without type information, or are we good here?

It's possible, but unlikely. I updated the callers of the interfaces I added type information to individually. I just slightly misunderstood the way this interface worked, because it's weirdly-implemented.
Flags: needinfo?(kmaglione+bmo)
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.