1510553 - The "." character is wrongly hyperlinked with the "Learn more" string from the "SNIPPETS_SEND_TO_DEVICE_TEST" snippet

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Marius Coman [:mcoman], Ecosystem QA]

Attachment: rec of the issue.gif

[Affected versions]:
- Firefox 64 and above

[Affected Platforms]:
- All Windows
- All Linux
- All Mac

[Prerequisites]:
- Have an RTL Firefox locale build (eg. Firefox AR) installed.
- Have a Firefox profile with the following prefs in the "about:config" page:
	- "browser.newtabpage.activity-stream.asrouterExperimentEnabled" with "true" value;

[Steps to reproduce]:
1. Open the browser with the profile from prerequisites.
2. Navigate to "about:config#asrouter" page.
3. Scroll down to the "SNIPPETS_SEND_TO_DEVICE_TEST" section and click the "Show" button.
4. Click the "Install now" button.
5. Observe the hyperlink from ".The intended recipient of the email must have consented. Learn more" string.

[Expected result]:
- Only the "Learn more" string is hyperlinked.

[Actual result]:
- The "." character is also hyperlinked.

[Additional Notes]:
- This issue is also reproducible on the other Firefox locales but is more visible on the RTL ones.
- This issue is not reproducible with the "Newsletter" or with the "Simple" snippets.
- Attached a screen recording of the issue.

Comment 1

[Punam Dahiya [:pdahiya]]

Able to replicate the issue using below STR

[Prerequisites]:
- Have an RTL Firefox locale build (eg. Firefox AR) installed.
- Have a Firefox profile with the following prefs in the "about:config" page:
	- "browser.newtabpage.activity-stream.asrouterExperimentEnabled" with "true" value;
        - Set "browser.newtabpage.activity-stream.asrouter.devtoolsEnabled" to true

[Steps to reproduce]:
1. Open the browser with the profile from prerequisites.
2. Navigate to "about:newtab#asrouter" page.
3. Scroll down to the "SNIPPETS_SEND_TO_DEVICE_TEST" section and click the "Show" button.
4. Click the "Install now" button.
5. Observe the hyperlink from ".The intended recipient of the email must have consented. Learn more" string.

Comment 2

[Punam Dahiya [:pdahiya]]

The issue appears to be period is part of the privacy link inside disclaimer html that's rendered inside anchor tag. 

https://github.com/mozilla/activity-stream/blob/master/lib/SnippetsTestMessageProvider.jsm#L154

Moving period out of the link fixes the issue. Will submit patch. Thanks!

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4588

Comment 4

[Punam Dahiya [:pdahiya]]

Fix merged in master https://github.com/mozilla/activity-stream/commit/cdae1bd3790767f33390bcad6a29f31b5ef171c4

Comment 5

[Ed Lee :Mardak]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta or can it ride the trains?

Comment 7

[Punam Dahiya [:pdahiya]]

We should be uplifting this to Beta, I will submit uplift request. Thanks!

Comment 8

[Punam Dahiya [:pdahiya]]

Hi Marius

Can you please verify this issue is fixed in nightly. This will help to submit uplift request to beta. Thanks!

Comment 9

[Punam Dahiya [:pdahiya]]

Attachment: Fix Bug 1510553 - [RTL] Dot character is wrongly hyperlinked with the Learn more string from the SNIPPETS_SEND_TO_DEVICE_TEST snippet r=andreio

[RTL] Dot character is wrongly hyperlinked with the Learn more string from the SNIPPETS_SEND_TO_DEVICE_TEST snippet

Comment 10

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly AR locale build (66.0a1 Build ID - 20181220215605) installed, on Windows 10 x64, Arch Linux and Mac 10.13.3. Now, the "." character is no longer hyperlinked.

Comment 11

[Punam Dahiya [:pdahiya]]

Try server run on beta

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8e10f50c2681f796f36837d47287505f6db1aef

Comment 12

[Punam Dahiya [:pdahiya]]

Comment on attachment 9032861 [details]
Fix Bug 1510553 - [RTL] Dot character is wrongly hyperlinked with the Learn more string from the SNIPPETS_SEND_TO_DEVICE_TEST snippet r=andreio

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1492088

User impact if declined: For Firefox RTL locales, 'Learn More' string shown in Send to device snippets at the bottom of new tab page will show dot wrongly hyperlinked.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): One line change that moves dot outside of link tag

String changes made/needed: none

Comment 13

[Punam Dahiya [:pdahiya]]

[Tracking Requested - why for this release]:

Comment 14

[Punam Dahiya [:pdahiya]]

Tracking Requested - Better user experience with low hanging UI fixes discovered by QA

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9032861 [details]
Fix Bug 1510553 - [RTL] Dot character is wrongly hyperlinked with the Learn more string from the SNIPPETS_SEND_TO_DEVICE_TEST snippet r=andreio

[Triage Comment]
Trivial change to end a link prior to the period. Approved for 65.0b7.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

I don't think this needs to be tracked since it's a super-minor UI polish issue.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/25eb46d5f2df

Comment 18

[Stefan [:StefanG_QA]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:65)Gecko/20100101 Firefox/65.0 (20181227144402)

I have verified this issue is no longer reproducible with the latest Firefox Beta AR locale build (65.0b7 Build ID - 20181220215605) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.14.2. Now, the "." character is no longer hyperlinked.