Closed
Bug 1385170
Opened 7 years ago
Closed 7 years ago
show button string by checking if the platform support set default browser in background
Categories
(Firefox :: New Tab Page, enhancement, P1)
Firefox
New Tab Page
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: gasolin, Assigned: gasolin)
References
(Depends on 1 open bug)
Details
(Whiteboard: [photon-onboarding])
Attachments
(1 file, 1 obsolete file)
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•7 years ago
|
Whiteboard: [ph
Assignee | ||
Updated•7 years ago
|
Flags: qe-verify+
Whiteboard: [ph → [photon-onboarding][triage]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years 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•7 years 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•7 years ago
|
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
Can we reuse the event from bug 1384045 here?
Assignee | ||
Comment 9•7 years ago
|
||
You mean 'beforeshow'? yes, though we still need extra event to communicate with onboarding_agent for UITour related actions.
Assignee | ||
Comment 10•7 years 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•7 years ago
|
Attachment #8891870 -
Attachment is obsolete: true
Attachment #8891870 -
Flags: review?(dtownsend)
Updated•7 years ago
|
QA Contact: jwilliams
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
Comment 14•7 years 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•7 years ago
|
||
Thanks! For QA please test on at least Windows 7 and Ubuntu to verify it works.
Keywords: checkin-needed
Comment 18•7 years 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d000d7961749
You need to log in
before you can comment on or make changes to this bug.
Description
•