Closed
Bug 1365887
Opened 7 years ago
Closed 7 years ago
Can't open resource:///modules/ from the location bar
Categories
(Firefox :: Address Bar, defect, P2)
Tracking
()
RESOLVED
FIXED
Firefox 55
People
(Reporter: Gijs, Assigned: tfeserver, Mentored)
References
Details
(Keywords: good-first-bug, regression, Whiteboard: [fxsearch][good first bug][lang=js])
Attachments
(1 file)
STR:
1. type "resource:///modules/ in the location bar and hit enter
ER:
load the file listing of modules
AR:
get redirected to the default search engine
Alternative STR:
1. type "resource://app/modules/" in the location bar and hit enter
ER + AR:
load the (same) file listing of modules
This is a regression, but I'm not sure when it broke. It's also broken on beta. I don't know that this is a location bar rather than a core bug, but let's start here for now.
Comment 1•7 years ago
|
||
I suspect the problem is here:
http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/toolkit/components/places/UnifiedComplete.js#1598
// Check the host, as "http:///" is a valid nsIURI, but not useful to us.
// But, some schemes are expected to have no host. So we check just against
// schemes we know should have a host. This allows new schemes to be
// implemented without us accidentally blocking access to them.
let hostExpected = new Set(["http", "https", "ftp", "chrome", "resource"]);
if (hostExpected.has(uri.scheme) && !uri.host)
return false;
resource://something has a host, resource:///something doesn't. Sounds like we should relax this check (by removing resource from the Set), and have a test for it, of course.
Priority: -- → P2
Whiteboard: [fxsearch]
Comment 2•7 years ago
|
||
The test could probably be added to toolkit/components/places/tests/unifiedcomplete/test_visit_url.js
Comment 4•7 years ago
|
||
Sure! I'll assign the bug when you attach the first patch.
For the basic introduction you can follow the guide on MDN:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
But feel free to ask questions, there's a needinfo flag at the bottom of the bug that you can set on me.
When adding the simple test:
do_print("visit url, resource:///modules/");
await check_autocomplete({
search: "resource:///modules/",
searchParam: "enable-actions",
matches: [ makeVisitMatch("resource:///modules/", "resource:///modules/", { heuristic: true }) ]
});
It throw me an error by comparing the strings:
moz-action:visiturl,{"url":"resource%3A%2F%2F%2Fmodules%2F","input":"resource%3A%2F%2F%2Fmodules%2F"}
and
resource:///modules/
Maybe you have a tip about that?
Flags: needinfo?(mak77)
All i can see, is that NetUtil.newURI() is not returning the expected value?
Comment 7•7 years ago
|
||
(In reply to tfe from comment #5)
> It throw me an error by comparing the strings:
> moz-action:visiturl,{"url":"resource%3A%2F%2F%2Fmodules%2F","input":
> "resource%3A%2F%2F%2Fmodules%2F"}
> and
> resource:///modules/
>
> Maybe you have a tip about that?
Before or after fixing the bug? since before fixing the bug it's expected you're getting a failure.
Otherwise, it's possible there's also another bug. if you launch the browser with ./mach run after fixing the code, does the location bar work as expected?
Flags: needinfo?(mak77)
Oh, i thought the bug was already fixed, and i only had to add the test.
After building, and accessing to the uri: resource:///modules/, it searches the string to my default search engine.
I'll look at how to fix the bug too then.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
With this patch the resource:///modules is now accessible.
I think i found another bug related with the resource://.
The "parent folder link" is not working at all.
May i fix it directly here, or in another ticket?
Flags: needinfo?(mak77)
Comment 11•7 years ago
|
||
(In reply to tfe from comment #10)
> May i fix it directly here, or in another ticket?
Another ticket please, it's unrelated to the Location Bar component.
Flags: needinfo?(mak77)
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8869108 [details]
Bug 1365887 - Access resource:// urls
https://reviewboard.mozilla.org/r/140736/#review144502
It looks good, thank you!
I started some Try builds to check tests, when those are done we can autoland this.
Attachment #8869108 -
Flags: review?(mak77) → review+
Comment 13•7 years ago
|
||
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/74890c4c6bc3
Access resource:// urls r=mak
Comment 14•7 years ago
|
||
Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b3da04fb9f6c300a31f465bac6aeafe9e464c6b6&tochange=99d514852ac5228c05301a9f1b9ad01c742b8bae
Marco Bonardo — Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw
Blocks: 1306639
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox53:
--- → affected
Keywords: regression
Version: 53 Branch → 51 Branch
Comment 15•7 years ago
|
||
The range is not exactly correct, in the sense that it is right based on what happens in the product, but basically bug 1306639 made the location bar respect what UnifiedComplete decides. The real bug was in unifiedComplete from far more time, probably from the beginning, but until bug 1306639 we were just ignoring this code. Btw, not that it really matters much, now it's fixed.
Comment 16•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 17•7 years ago
|
||
Is this something that can ride the 55 train or should we consider it for backport?
Assignee: nobody → tfeserver
status-firefox-esr52:
--- → affected
Flags: needinfo?(mak77)
Flags: in-testsuite+
Comment 18•7 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> Is this something that can ride the 55 train or should we consider it for
> backport?
I don't think it's useful to uplift it, this is something mostly for us (firefox devs), not that much for the end users.
Flags: needinfo?(mak77)
You need to log in
before you can comment on or make changes to this bug.
Description
•