Closed
Bug 1455304
Opened 6 years ago
Closed 6 years ago
macOS "Share" > "Add to reading list" opens Safari
Categories
(Firefox :: Address Bar, enhancement)
Tracking
()
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: alberts, Assigned: daleharvey)
References
Details
Attachments
(1 file)
After the macOS "Share" feature got implemented (bug 1363168 - congrats!), when selecting "..." (page action) > "Share" > "Add to reading list" - I was hoping for a Firefox reading list. Instead Safari opened and that was it. I checked and found the URL in Safari's reading list. When selecting it again on a different tab/URL I get no feedback, though the item was added to Safari's reading list as well. So there are two problems here: 1) In the current form this item doesn't provide the Firefox user enough feedback what just happened 2) I'd expect it to be a Firefox internal reading list - ideally one I can share on all devices
Comment 1•6 years ago
|
||
You're right, Albert. I think we should filter this sharing option out for the time being, because Pocket integration is placed somewhere else. Aaron, does that make sense to you?
Flags: needinfo?(abenson)
Comment 2•6 years ago
|
||
Absolutely. Filtering out that item would be ideal.
Flags: needinfo?(abenson)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → dharvey
Assignee | ||
Comment 3•6 years ago
|
||
hrm, I cant think of any other way than to filter these our by name, I dont think thats ideal but I am also not seeing these being translated if I set my mac to any other language. We can just have a blacklist of names that include translations if we find any?
Assignee | ||
Comment 4•6 years ago
|
||
* filter these out than by name
Comment 5•6 years ago
|
||
(In reply to Dale Harvey (:daleharvey) from comment #3) > hrm, I cant think of any other way than to filter these our by name, I dont > think thats ideal but I am also not seeing these being translated if I set > my mac to any other language. We can just have a blacklist of names that > include translations if we find any? I think that'd be OK for now. Perhaps QA can find possible translations? Oh and can you put the filtering in the Mac SharingService? I know that string matching in C++ is more of a pain than in JS, but it does 'feel' like the best place to me.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•6 years ago
|
||
Heh funnily enough I was looking around code for another bug and noticed chromium doing the same thing as this patch @ https://codereview.chromium.org/2950403002/diff/100001/chrome/browser/ui/cocoa/share_menu_controller.mm?_ga=2.6916466.1529169875.1524493973-1529565179.1524493973#newcode66
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8970085 [details] Bug 1455304 - Filter unwanted sharing providers. https://reviewboard.mozilla.org/r/238860/#review244616 ::: widget/cocoa/nsMacSharingService.mm:58 (Diff revision 1) > NSArray* sharingService = [NSSharingService > sharingServicesForItems:[NSArray arrayWithObject:url]]; > int32_t serviceCount = 0; > > for (NSSharingService *currentService in sharingService) { > - > + if (![blackList containsObject: currentService.title]) { No space after colon, and it looks like you forgot to rename this to filteredProviderTitles.
Attachment #8970085 -
Flags: review?(mstange) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•6 years ago
|
||
Thanks and sorry for the silly mistake, pushed to try @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbe2dd8e22d814cf10ed8c893c66727bc5064e3a
Comment 12•6 years ago
|
||
Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b94268b39eca Filter unwanted sharing providers. r=mstange
Comment 13•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b94268b39eca
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in
before you can comment on or make changes to this bug.
Description
•