Closed Bug 1527701 Opened 2 years ago Closed 2 years ago

Update DSMessage and Navigation links to use SafeAnchor

Categories

(Firefox :: New Tab Page, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 67
Iteration:
67.1 - Jan 28 - Feb 10
Tracking Status
firefox66 + verified
firefox67 --- verified

People

(Reporter: pdahiya, Assigned: pdahiya)

References

Details

(Keywords: github-merged)

Attachments

(2 files)

Links inside DSMessage and Navigation discovery stream components should be updated to use Safe Anchor tag.

Assignee: nobody → pdahiya
Iteration: --- → 67.1 - Jan 28 - Feb 10
Depends on: 1524669
Priority: -- → P1

[Tracking Requested - why for this release]: Follow up of security feedback bug 1524669, its covering protocol check on anchor link for navigation and message discovery stream components.

Keywords: github-merged

How to test using test feed:

  1. go to about:config

  2. search for discoverystream.config and set preference to {"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://gist.githubusercontent.com/punamdahiya/44f78e2f338cbf95f5c28d31faed4639/raw/1c61974ca8ebd02431512d9425031e3a9c8e04f4/test-message-feed.json"}

  3. Open new tab and search for text 'Must Reads' in navigation component and it should not be linked.

  4. Search for 'Learn more' to find rendered message component and Learn More should not be linked.

  5. Search for text 'Health' in navigation component and it should not be linked.

  6. Open test feed in new tab
    https://gist.githubusercontent.com/punamdahiya/44f78e2f338cbf95f5c28d31faed4639/raw/1c61974ca8ebd02431512d9425031e3a9c8e04f4/test-message-feed.json

  7. Search for navigation item with title 'Must Reads' and url should be 'javascript://getpocket.com/explore/must-reads?src=fx_new_tab'

  8. Search for text 'Learn more' in json feed and respective link_url should be 'javascript://getpocket.com/firefox/new_tab_learn_more'

  9. Search for text 'health' in json feed and respective url should be 'ftp://getpocket.com/explore/health?src=fx_new_tab'

Blocks: 1528119
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Brahmini, can you QA?

Flags: needinfo?(bnagabandi)

Comment on attachment 9044310 [details]
Bug 1527701 - Use SafeAnchor in navigation and message component

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1512725

User impact if declined

Patch implements security feedback and ensures at UI end that links shown on message and navigation component in pocket new tab experiments are using protocol http[s].

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

See https://bugzilla.mozilla.org/show_bug.cgi?id=1527701#c3

List of other uplifts needed

Bug 1524669

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

This patch touches message and navigation components for pocket new tab discovery experiment that's not turned on by default in beta

String changes made/needed

None

Attachment #9044310 - Flags: approval-mozilla-beta?

Comment on attachment 9044310 [details]
Bug 1527701 - Use SafeAnchor in navigation and message component

Planned work for pocket/new tab.
I'm ok with this being verified after uplift. Let's get all this into beta 9.

Landing order: bug 1519879, bug 1525494, bug 1526861, bug 1524669, bug 1527195, bug 1525391, bug 1527347, bug 1525366, bug 1527626, bug 1527397, bug 1518258, bug 1527701, bug 1527370.

Attachment #9044310 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Punam Dahiya [:pdahiya] from comment #3)

How to test using test feed:

  1. go to about:config

  2. search for discoverystream.config and set preference to {"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://gist.githubusercontent.com/punamdahiya/44f78e2f338cbf95f5c28d31faed4639/raw/1c61974ca8ebd02431512d9425031e3a9c8e04f4/test-message-feed.json"}

  3. Open new tab and search for text 'Must Reads' in navigation component and it should not be linked.

  4. Search for 'Learn more' to find rendered message component and Learn More should not be linked.

  5. Open test feed in new tab
    https://gist.githubusercontent.com/punamdahiya/44f78e2f338cbf95f5c28d31faed4639/raw/1c61974ca8ebd02431512d9425031e3a9c8e04f4/test-message-feed.json

  6. Search for navigation item with title 'Must Reads' and url should be 'javascript://getpocket.com/explore/must-reads?src=fx_new_tab'

  7. Search for text 'Learn more' in json feed and respective link_url should be 'javascript://getpocket.com/firefox/new_tab_learn_more'

Tested on:

FF Nightly version : 67.0a1 (2019-02-15)
OS : Mac and Windows 10 Pro

I'm seeing that the link for Health in navigation is missing.
Can you confirm - should we expect link or not?

Recording : https://www.dropbox.com/s/owhpuo9krp2xefo/bug%201527701%20%28%3F%29.mp4?dl=0

Flags: needinfo?(bnagabandi) → needinfo?(pdahiya)

Yes, it's expected that Health does not open a link. It's pointing at "ftp://getpocket.com/…"

Considering latest QA steps, below are the test results.

Tested on :

FF Nightly version : 67.0a1 (2019-02-15)
OS : Mac and Windows 10 Pro

  • From Navigation component - Must Reads, Learn more and Health are not linked.
  • Seeing expected urls for the items from Json feed :
    • Must Reads - javascript://getpocket.com/explore/must-reads?src=fx_new_tab
    • Learn more - javascript://getpocket.com/firefox/new_tab_learn_more
    • Health - ftp://getpocket.com/explore/health?src=fx_new_tab

Works as expected.

Closing as verified.

Flags: needinfo?(pdahiya)
Status: RESOLVED → VERIFIED

(In reply to Brahmini Nagabandi from comment #12)

Considering latest QA steps, below are the test results.

Tested on :

FF Nightly version : 67.0a1 (2019-02-15)
OS : Mac and Windows 10 Pro

  • From Navigation component - Must Reads, Learn more and Health are
    not linked.
  • Seeing expected urls for the items from Json feed :
    • Must Reads -
      javascript://getpocket.com/explore/must-reads?src=fx_new_tab
    • Learn more - javascript://getpocket.com/firefox/new_tab_learn_more
    • Health - ftp://getpocket.com/explore/health?src=fx_new_tab

Works as expected.

Closing as verified.

Can you please verify this issue on Firefox 66 Beta 9 (https://archive.mozilla.org/pub/firefox/candidates/66.0b9-candidates/build1/)?

Flags: qe-verify+
Flags: needinfo?(bnagabandi)
Whiteboard: [qa-triaged]

BETA Testing:

QA Results:

Tested on :

FF Beta version : 66.0b9 (64-bit)
OS : Mac and Windows 10 Pro
Date : Feb 21

Verified on the latest Beta. Works as expected.

Marking as verified.

Flags: needinfo?(bnagabandi)

(In reply to Brahmini Nagabandi from comment #14)

BETA Testing:

QA Results:

Tested on :

FF Beta version : 66.0b9 (64-bit)
OS : Mac and Windows 10 Pro
Date : Feb 21

Verified on the latest Beta. Works as expected.

Marking as verified.

Thanks for verifying this!

Flags: qe-verify+
QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.