Closed
Bug 1322565
Opened 8 years ago
Closed 7 years ago
Remove gecko.handlerService.allowRegisterFromDifferentHost
Categories
(Firefox :: File Handling, defect, P5)
Firefox
File Handling
Tracking
()
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: Gijs, Assigned: iulian.radu67, Mentored)
Details
(Keywords: good-first-bug, Whiteboard: [good first bug])
Attachments
(1 file, 1 obsolete file)
2.33 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
I don't believe this preference should ever be set to true, so we should remove it.
Updated•7 years ago
|
Priority: -- → P5
Whiteboard: [good first bug]
Reporter | ||
Updated•7 years ago
|
Mentor: paolo.mozmail, gijskruitbosch+bugs
Keywords: good-first-bug
Assignee | ||
Comment 1•7 years ago
|
||
Hello, I hope this is what you had in mind. Please let me know how I can improve it as it is one of my first few patches.
Attachment #8819100 -
Flags: review?(gijskruitbosch+bugs)
Reporter | ||
Comment 2•7 years ago
|
||
Comment on attachment 8819100 [details] [diff] [review] Remove gecko.handlerService.allowRegisterFromDifferentHost Review of attachment 8819100 [details] [diff] [review]: ----------------------------------------------------------------- Perfect, thanks!
Attachment #8819100 -
Flags: review?(gijskruitbosch+bugs) → review+
Reporter | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6e2d96c1274 Remove gecko.handlerService.allowRegisterFromDifferentHost. r=gijs
Keywords: checkin-needed
Comment 4•7 years ago
|
||
backed out for eslint failure like https://treeherder.mozilla.org/logviewer.html#?job_id=40693746&repo=mozilla-inbound
Flags: needinfo?(gijskruitbosch+bugs)
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e04a75c36ec3 Backed out changeset a6e2d96c1274 for eslint failure
Reporter | ||
Comment 6•7 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #2) > Comment on attachment 8819100 [details] [diff] [review] > Remove gecko.handlerService.allowRegisterFromDifferentHost > > Review of attachment 8819100 [details] [diff] [review]: > ----------------------------------------------------------------- > > Perfect, thanks! So, eslint decided this wasn't quite perfect yet: [task 2016-12-16T15:35:14.084100Z] TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/components/feeds/WebContentConverter.js:159:9 | 'pb' is assigned a value but never used. (no-unused-vars) >diff --git a/browser/components/feeds/WebContentConverter.js b/browser/components/feeds/WebContentConverter.js > // We also reject handlers registered from a different host (see bug 402287) > // The pref allows us to test the feature > let pb = Services.prefs; >- if (!pb.getBoolPref(PREF_ALLOW_DIFFERENT_HOST) && >- (!["http:", "https:"].includes(aContentWindow.location.protocol) || >- aContentWindow.location.hostname != uri.host)) { >+ if (!["http:", "https:"].includes(aContentWindow.location.protocol) || >+ aContentWindow.location.hostname != uri.host) { Can you also remove the assignment to 'pb', and for good measure, the comment line > // The pref allows us to test the feature because we no longer test with this pref? Thanks!
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(iulian.radu67)
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → iulian.radu67
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•7 years ago
|
||
Oh, indeed. I now know about "mach lint" and I'll make sure to run it on future patches. Please let me know if I have set the right flags (review ?, needinfo).
Attachment #8819100 -
Attachment is obsolete: true
Flags: needinfo?(iulian.radu67) → needinfo?(gijskruitbosch+bugs)
Attachment #8819368 -
Flags: review?(gijskruitbosch+bugs)
Reporter | ||
Comment 8•7 years ago
|
||
Comment on attachment 8819368 [details] [diff] [review] Bug1322565.patch Review of attachment 8819368 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/feeds/WebContentConverter.js @@ +155,5 @@ > } > > // We also reject handlers registered from a different host (see bug 402287) > + if (!["http:", "https:"].includes(aContentWindow.location.protocol) || > + aContentWindow.location.hostname != uri.host) { Nit: the second line should be aligned with the "!" so one space to the left.
Attachment #8819368 -
Flags: review?(gijskruitbosch+bugs) → review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ebcba38b2a82 Remove gecko.handlerService.allowRegisterFromDifferentHost pref, r=gijs
Reporter | ||
Comment 10•7 years ago
|
||
(In reply to Iulian Radu from comment #7) > Please let me know if I have set the right flags (review ?, > needinfo). review was good, needinfo wasn't necessary, but it didn't hurt. :-)
Flags: needinfo?(gijskruitbosch+bugs)
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ebcba38b2a82
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in
before you can comment on or make changes to this bug.
Description
•