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

VERIFIED FIXED in Firefox 17

Status

defect
VERIFIED FIXED
7 years ago
5 months ago

People

(Reporter: markh, Assigned: markh)

Tracking

unspecified
Firefox 19
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17 verified, firefox18 verified)

Details

(Whiteboard: [fx17])

Attachments

(1 attachment)

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]?
Duplicate of this bug: 805257
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+
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
Closed: 7 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+
Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.