Can't open resource:///modules/ from the location bar

RESOLVED FIXED in Firefox 55

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Gijs, Assigned: tfeserver, Mentored)

Tracking

({good-first-bug, regression})

51 Branch
Firefox 55
good-first-bug, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [fxsearch][good first bug][lang=js])

Attachments

(1 attachment)

59 bytes, text/x-review-board-request
mak
: review+
Details
(Reporter)

Description

2 years ago
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.
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]
The test could probably be added to toolkit/components/places/tests/unifiedcomplete/test_visit_url.js
Mentor: mak77
Keywords: good-first-bug
Whiteboard: [fxsearch] → [fxsearch][good first bug][lang=js]
(Assignee)

Comment 3

2 years ago
Hello. Can i work on this ticket?
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.
(Assignee)

Comment 5

2 years ago
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)
(Assignee)

Comment 6

2 years ago
All i can see, is that NetUtil.newURI() is not returning  the expected value?
(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)
(Assignee)

Comment 8

2 years ago
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

2 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)
(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

2 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 14

2 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
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/74890c4c6bc3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Is this something that can ride the 55 train or should we consider it for backport?
Assignee: nobody → tfeserver
status-firefox53: affected → wontfix
status-firefox-esr52: --- → affected
Flags: needinfo?(mak77)
Flags: in-testsuite+
(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.
status-firefox54: affected → wontfix
status-firefox-esr52: affected → wontfix
Flags: needinfo?(mak77)
You need to log in before you can comment on or make changes to this bug.