Restrict pages which can be shared to http and https protocols.

VERIFIED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

unspecified
Firefox 19
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17 verified, firefox18 verified)

Details

(Whiteboard: [fx17])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Currently any URL can be shared, including "file://", "chrome://" and "data:" URLs.  We possibly want to restrict what can be shared to, eg, http://, https:// ftp:// and ftps:// URLs.
Sharing of about: URIs (e.g. about:home) should also be prevented.
Whiteboard: [fx17]?
(Assignee)

Updated

5 years ago
Duplicate of this bug: 805257
(Assignee)

Comment 3

5 years ago
Created attachment 674968 [details] [diff] [review]
hide share button for non http/https/ftp/ftps urls

This patch keeps the share button hidden if the current URI scheme isn't one of http, https, ftp, ftps (which is the list we used for F1, and seems reasonable to me - any others needed?)
Assignee: nobody → mhammond
Attachment #674968 - Flags: review?(jaws)
Attachment #674968 - Flags: review?(gavin.sharp)
Comment on attachment 674968 [details] [diff] [review]
hide share button for non http/https/ftp/ftps urls

Review of attachment 674968 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser-social.js
@@ +508,5 @@
>    dismissUnsharePopup: function SSB_dismissUnsharePopup() {
>      this.unsharePopup.hidePopup();
>    },
>  
> +  canShareCurrentUrl: function SSB_canShareCurrentUrl(aURI) {

s/canShareCurrentUrl/canSharePage/

@@ +510,5 @@
>    },
>  
> +  canShareCurrentUrl: function SSB_canShareCurrentUrl(aURI) {
> +    let tabBrowser = gBrowser.getBrowserForTab(gBrowser.selectedTab);
> +    let uri = tabBrowser.currentURI;

These two lines aren't needed, since you can just use the |aURI| argument of this function :)

@@ +513,5 @@
> +    let tabBrowser = gBrowser.getBrowserForTab(gBrowser.selectedTab);
> +    let uri = tabBrowser.currentURI;
> +    // We only allow sharing of http, https, ftp, ftps.
> +    return (uri && (uri.schemeIs('http') || uri.schemeIs('https') ||
> +                    uri.schemeIs('ftp') || uri.schemeIs('ftps')));

Does FB actually allow you to share ftp/ftps links? I think we should just limit this to http/https. If we're not sure, I think it is better to err on the side of less abilities, since we can add those in the future, but it would be harder to remove ones in the future.

@@ +521,5 @@
>      let shareButton = this.shareButton;
>      if (shareButton)
>        shareButton.hidden = !Social.uiVisible || this.promptImages == null ||
> +                           !SocialUI.haveLoggedInUser() ||
> +                           !this.canShareCurrentUrl();

!this.canSharePage(gBrowser.currentURI);
Attachment #674968 - Flags: review?(jaws)
Attachment #674968 - Flags: review?(gavin.sharp)
Attachment #674968 - Flags: review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/081340dcc074
(Assignee)

Updated

5 years ago
Summary: Restrict pages which can be shared to ones with "normal" protocols. → Restrict pages which can be shared to http and https protocols.
https://hg.mozilla.org/mozilla-central/rev/081340dcc074
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Comment on attachment 674968 [details] [diff] [review]
hide share button for non http/https/ftp/ftps urls

[Triage Comment]
let's get this social-only fix in for 17
Attachment #674968 - Flags: approval-mozilla-beta+
Attachment #674968 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ffee7552efec
https://hg.mozilla.org/releases/mozilla-beta/rev/3999273bf1ca
status-firefox17: --- → fixed
status-firefox18: --- → fixed
Whiteboard: [fx17]? → [fx17]
Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
status-firefox18: fixed → verified
You need to log in before you can comment on or make changes to this bug.