Closed
Bug 1498934
Opened 6 years ago
Closed 6 years ago
Opening magnet links no longer works
Categories
(Firefox :: File Handling, defect, P1)
Firefox
File Handling
Tracking
()
VERIFIED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox62 | --- | unaffected |
firefox63 | + | fixed |
firefox64 | + | verified |
firefox65 | --- | verified |
People
(Reporter: marco, Assigned: kmag)
References
Details
(Keywords: regression)
Attachments
(1 file)
46 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
pascalc
:
approval-mozilla-release+
|
Details | Review |
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 | ||
Comment 1•6 years ago
|
||
Requesting tracking as this is a recent regression, with a clear regression range, in an user-visible feature.
tracking-firefox63:
--- → ?
tracking-firefox64:
--- → ?
Updated•6 years ago
|
Comment 2•6 years 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•6 years 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•6 years 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®exp=false&path=*.js
Assignee | ||
Comment 6•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/10476448b17b53befefdf4eeb6d7278bd02401a9
Bug 1498934: Fix handler app enumeration issue. r=Gijs
Comment 7•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment 8•6 years ago
|
||
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•6 years 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 11•6 years ago
|
||
Note, this probably also fixes bug 1498932, which is easily reproducible in a clean profile.
Comment 12•6 years 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.
Comment 13•6 years 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®exp=false&path=*.js
Flags: needinfo?(kmaglione+bmo)
Comment 14•6 years ago
|
||
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•6 years ago
|
||
bugherder uplift |
Comment 16•6 years 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•6 years ago
|
Flags: needinfo?(mcastelluccio)
Comment 17•6 years ago
|
||
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•6 years ago
|
||
bugherder uplift |
Comment 19•6 years 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•6 years 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•6 years 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)
Updated•6 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•