1737854 - macOS 'Share' menu modifies URL such that shared page doesn't load

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Sam Johnson]

According to mozregression, bug 1722758 causes the Share menu to send URLs that are broken--for example, sharing a Bugzilla bug results in the bug not loading on the target device.

Steps to reproduce:

1. Go to any Bugzilla bug such as this one.
2. Right-click the tab > Share > AirDrop
3. Select a device to AirDrop to (can be an iPhone running Safari, another Mac running Firefox, doesn't matter).
4. The shared bug does not load properly on the target device due to the equals sign being changed.

Note that this issue does not occur before bug 1722758, and also does not occur when sharing the same URL from Safari.

Mozregression output from the regressing build:
Bug 1722758 - Patch 4 - Encode additional characters in the URL required for NSURL compatiblity r=mac-reviewers,mstange

Perform additional encoding of URLs before converting to an NSURL as required per the NSURL URLWithString: method documentation.

Differential Revision: https://phabricator.services.mozilla.com/D122653

Comment 1

[Haik Aftandilian [:haik]]

Thanks for the report. I'm working on a fix. My fix for bug 1722758 regressed this by encoding some characters including = in the query segment of the URL where they should not be encoded.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1722758

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1722758

Comment 5

[Markus Stange [:mstange]]

This also affects the handoff implementation from bug 1525788. For example, handing off bugzilla tabs doesn't work because the equals sign in the URL doesn't survive the encoding.

Comment 6

[Haik Aftandilian [:haik]]

This is a serious regression. I'm planning to post a fix with more tests that addresses bug 1737854 but is more limited in scope by handling escapes properly in the ref URL component only, but will need a follow up fix to do all the other components correctly and also address bug 1739533. Different URL components need different encoding rules.

Another option is to backout the fix for 1722758 and aim for that to be included in a dot release, pending approval from the release managers.

This wasn't caught by the existing tests (that were modified in bug 1722758) because they make sure that NSURL accepts the encoded URL, but not that the encoded URL was correct which was a known limitation. And I didn't realize encoding when it isn't necessary would be a problem.

We might be able to address this using [NSString stringByAddingPercentEncodingWithAllowedCharacters:] and character sets such as URLPathAllowedCharacterSet to fix this with less special case code.

Comment 7

[Haik Aftandilian [:haik]]

Attachment: Bug 1737854 - macOS 'Share' menu modifies URL such that shared page doesn't load r=mstange! r=#necko-reviewers!

Revert some of the fix for 1722758 so that only the URL ref component is re-encoded for NSURL compatibility. Other URL fields need additional work to be addressed in a follow up.

Update the set of characters re-encoded to be as minimal as possible and include missing characters.

Add tests to ensure encoding works as expected, not just that it is accepted by NSURL.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2aa7d637d51
macOS 'Share' menu modifies URL such that shared page doesn't load  r=necko-reviewers,mstange,valentin

Comment 9

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/f2aa7d637d51

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:haik, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Haik Aftandilian [:haik]]

Comment on attachment 9249382 [details]
Bug 1737854 - macOS 'Share' menu modifies URL such that shared page doesn't load r=mstange! r=#necko-reviewers!

Beta/Release Uplift Approval Request

• User impact if declined: Users that use the File->Share menu option or right-click on tab->Share option to share a URL to another application on macOS such as Mail or Messages will end up with incorrect URL being passed to the other application.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See description of bug, but it is not required to use Air Drop. If the user is signed in with an Apple ID, sharing to Messages or Mail and verifying the URL is correct might be easier.

We also want to make sure the fix for bug 1722758 has not regressed with this fix.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix is Mac-specific. The fix restores the encoding state of other fields in the URL and only affects the ref component (also known as the fragment identifier or the part after the first '#') so that the encoding used for the Share option is restored to how it has been for years. Only the ref component will now have the extra encoding for the Share menu until bug 1740565 is fixed in the future to address all URL components.
• String changes made/needed:

Comment 12

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue with Firefox 95.0a1 (20211026214417) while using AirDrop between two macOS 10.15 machines. Using STR from comment 0 the page is not opened successfully on the shared macOS 10.15 system.

The AirDrop share works as expected with Firefox 96.0a1 (20211116212601) when sharing tabs from macOS 10.15 and macOS 11.6 ARM to macOS 10.15 machine. The links are properly loaded after following STR from comment 0. Also sharing links like https://chat.mozilla.org/#/room/#macdev:mozilla.org/ https://chat.mozilla.org/#/room/#crashreporting:mozilla.org from bug 1722758 are properly opened on the other macOS 10.15 machines when shared from macOS 10.15 and 11.6 ARM.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9249382 [details]
Bug 1737854 - macOS 'Share' menu modifies URL such that shared page doesn't load r=mstange! r=#necko-reviewers!

Approved for 95 beta 9, thanks.

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/ca46d8972d7a

Comment 15

[Alexandru Trif, Desktop QA [:atrif]]

Verified fixed with 95.0b9 (20211118185700) when sharing using AirDrop from macOS 10.15 and macOS 11.6 ARM to another macOS 10.15 machine. The links are loaded correctly on the shared machine.