show button string by checking if the platform support set default browser in background

VERIFIED FIXED in Firefox 56

Status

()

Firefox
New Tab Page
P1
normal
VERIFIED FIXED
9 months ago
9 months ago

People

(Reporter: gasolin@mozilla.com, Assigned: gasolin@mozilla.com)

Tracking

(Depends on: 1 bug)

unspecified
Firefox 56
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [photon-onboarding])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
In UI spec it described only windows 7 allow set default browser in background, but after QA test in bug 1374717 comment 35 we found Ubuntu also support set default browser in background. That means only checking windows7 is not the right way to go.

We could show the correct string by checking UITour's canSetDefaultBrowserInBackground property for on the getConfiguration("appInfo") response
(Assignee)

Updated

9 months ago
Whiteboard: [ph
(Assignee)

Updated

9 months ago
Flags: qe-verify+
Whiteboard: [ph → [photon-onboarding][triage]
(Assignee)

Updated

9 months ago
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 3

9 months ago
The PR also fix the issue on window 7, which will show the disabled button instead of 2 lines of alternative message after user click the button.

The expect result should be showing 2 lines of alternative message after user click the button.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

9 months ago
The PR send an event when page is rendered and listen in default-browser-tour's getPage method.
Once event is received, it will send another event to ask agent doing UITour check


The previous PR create the extra `processPage` method to call the action after page is rendered,
https://reviewboard.mozilla.org/r/162406/diff/3#index_header

But after sync with Rex, we think listen event in `getPage` might be more flexible than make calls on every entry-point.

Updated

9 months ago
Target Milestone: --- → Firefox 56

Updated

9 months ago
Priority: -- → P1
Comment hidden (mozreview-request)
Can we reuse the event from bug 1384045 here?
(Assignee)

Comment 9

9 months ago
You mean 'beforeshow'? yes, though we still need extra event to communicate with onboarding_agent for UITour related actions.
(Assignee)

Comment 10

9 months ago
I'm a bit concerned about listening on the same `beforeshow` event in `gotoPage` function since it might be too late for async showing the button text.

But I found no visual regression when I test it on Macbook Air. So I'll remove my custom event and reuse `beforeshow` event from bug 1384045
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8891870 - Attachment is obsolete: true
Attachment #8891870 - Flags: review?(dtownsend)
(Assignee)

Updated

9 months ago
Depends on: 1384045

Updated

9 months ago
QA Contact: jwilliams
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]

Comment 14

9 months ago
mozreview-review
Comment on attachment 8891191 [details]
Bug 1385170 - show button string by checking if the platform support set default browser in background;

https://reviewboard.mozilla.org/r/162406/#review168484
Attachment #8891191 - Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 17

9 months ago
Thanks! 

For QA please test on at least Windows 7 and Ubuntu to verify it works.
Keywords: checkin-needed

Comment 18

9 months ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d000d7961749
show button string by checking if the platform support set default browser in background;r=mossop
Keywords: checkin-needed

Comment 19

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d000d7961749
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
I have verified that this is fixed.
Status: RESOLVED → VERIFIED
Depends on: 1389558
You need to log in before you can comment on or make changes to this bug.