Closed Bug 1264623 Opened 4 years ago Closed 4 years ago

Allow for a leading slash in the path to web_accessible_resources

Categories

(WebExtensions :: Untriaged, defect, P2)

defect

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.3 - Apr 25
Tracking Status
firefox48 --- fixed

People

(Reporter: bsilverberg, Assigned: bsilverberg)

Details

(Whiteboard: triaged)

Attachments

(1 file)

This comes from a discussion in bug 1207394 [1]. Chrome allows the path to web_accessible_resources to contain a leasing slash, so we should probably do the same.

I have a patch ready for this which I will submit soon. 

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1207394#c15
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Iteration: --- → 48.3 - Apr 25
Attachment #8741345 - Flags: review?(kmaglione+bmo)
Comment on attachment 8741345 [details]
MozReview Request: Bug 1264623 - Allow for a leading slash in the path to web_accessible_resources, r?kmag

https://reviewboard.mozilla.org/r/46403/#review43145

::: toolkit/components/extensions/Extension.jsm:1155
(Diff revision 1)
>          whitelist.push(perm);
>        }
>      }
>      this.whiteListedHosts = new MatchPattern(whitelist);
>  
> -    this.webAccessibleResources = new MatchGlobs(manifest.web_accessible_resources || []);
> +    // strip leading slashes from web_accessible_resources

Please capitalize.

::: toolkit/components/extensions/Extension.jsm:1164
(Diff revision 1)
> +        if (path.length > 0 && path[0] == "/") {
> +          path = path.substr(1);
> +        }
> +        return path;
> +      });
> +    }

This would be much simpler as something like `resources.map(path => path.replace(/^\/+/, ""))`
Comment on attachment 8741345 [details]
MozReview Request: Bug 1264623 - Allow for a leading slash in the path to web_accessible_resources, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46403/diff/1-2/
Attachment #8741345 - Flags: review?(kmaglione+bmo)
Attachment #8741345 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8741345 [details]
MozReview Request: Bug 1264623 - Allow for a leading slash in the path to web_accessible_resources, r?kmag

https://reviewboard.mozilla.org/r/46403/#review43231
I see the try run includes a failure of  toolkit/components/extensions/test/mochitest/test_ext_notifications.html on Windows 7, but that is reported as an intermittent already, so I don't think it has anything to do with this patch.
Keywords: checkin-needed
Priority: -- → P2
Whiteboard: triaged
https://hg.mozilla.org/mozilla-central/rev/c3c49869ef0c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.