1377439 - Should adapt the oboarding UI to the hight-contrast display mode

Status: VERIFIED FIXED

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

Description

[Fischer [:Fischer]]

SOP:
- Set `browser.display.document_color_use=2` to enable the high-contrast mode
- Open about:newtab or about:home

Expected result:
The images of the Onboarding tours exist

Actual result:
The images of the Onboarding tours disappear

This is because in high-contrast mode the background images specified by author stylesheet would be ignored. See [1] for details.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1022553#c2

Comment 1

[Fischer [:Fischer]]

Attachment: not_handle_high_contrast_mode_1.png

Comment 2

[Fischer [:Fischer]]

Attachment: not_handle_high_contrast_mode_2.png

Comment 3

[Fischer [:Fischer]]

Attachment: handle_high_contrast_mode_when_disabling_high_contrast_mode_1.png

Comment 4

[Fischer [:Fischer]]

Attachment: handle_high_contrast_mode_when_disabling_high_contrast_mode_2.png

Comment 5

[Fischer [:Fischer]]

Attachment: handle_high_contrast_mode_when_enabling_high_contrast_mode_1.png

Comment 6

[Fischer [:Fischer]]

Attachment: handle_high_contrast_mode_when_enabling_high_contrast_mode_2.png

Comment 7

[Fischer [:Fischer]]

Attachment: handle_high_contrast_mode.gif

Comment 8

[Fischer [:Fischer]]

Hi Verdi,
Per the discussion during the SF workweek, we should make the most important and the minimum UI ingredients visible under the high-contrast display mode. Please see the UI for the high-contrast display mode:
- attachment 8884155 [details]: handle_high_contrast_mode_when_enabling_high_contrast_mode_1.png
- attachment 8884156 [details]: handle_high_contrast_mode_when_enabling_high_contrast_mode_2.png
- attachment 8884159 [details]: handle_high_contrast_mode.gif

You should see
- buttons now would have outline
- the upper-left fox icon is visible
- the X close buttons on the notification bar and on the overlay are visible
- the navigation item of the current tour on the overlay's right side would have outline
- the hovered navigation item on the overlay's right side would have outline

Please confirm the UI. If ok, we will go for this implementation, thank you.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Are you sure this is not 56?

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ni for above question.

Comment 11

[Fischer [:Fischer]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #10)
> Ni for above question.
Thanks for updated this. I guess this was a mistake while creating bugs.

Comment 12

[Verdi [:verdi] Best to slack me]

This is looking good.

I have one concern about the buttons "Learn More" (in the notifications) and "Show Private Browsing in Menu", etc. in the overlay. I think they can be higher contrast. Either black or maybe they should be blue like links and other web buttons are (https://www.mozilla.org/en-US/firefox/new/ for example). I'm not sure of the convention here. Can we check with Yura?

Comment 13

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Verdi [:verdi] from comment #12)
> This is looking good.
> 
> I have one concern about the buttons "Learn More" (in the notifications) and
> "Show Private Browsing in Menu", etc. in the overlay. I think they can be
> higher contrast. Either black or maybe they should be blue like links and
> other web buttons are (https://www.mozilla.org/en-US/firefox/new/ for
> example). I'm not sure of the convention here. Can we check with Yura?

Yeah I would agree that black would make it great. There are a11y guidelines that are related to contrast and text, but not so much to things like borders and background. I guess since it's a high contrast mode, it we should ideally avoid lower contrast for those buttons (it looks like it would also be pretty consistent with the tab list buttons on the left). Here's a checker that might be useful for checking contrast (I guess for high contrast we would need AAA pass): http://webaim.org/resources/contrastchecker/

Comment 14

[Fischer [:Fischer]]

Attachment: button_grey_outline_under_hight_contrast_mode.png

Comment 15

[Fischer [:Fischer]]

Attachment: div_as_button_black_outline_under_hight_contrast_mode.png

Comment 16

[Fischer [:Fischer]]

It seems the grey border for a <button> under the high-contrast mode is the default style. Even set black border on the buttons in the bottom of the about:home page, we still got the grey border under the high-contrast mode. See attachment 8885683 [details]: button_grey_outline_under_hight_contrast_mode.png.

A simple solution is to utilize a <div> as a <button>. If that is a <div>, then the border would be black, which is clearer and more contrast, under the high-contrast mode. See attachment 8885684 [details]: div_as_button_black_outline_under_hight_contrast_mode.png.

Comment 17

[Yura Zenevich [:yzen] (please use "needinfo")]

yes please do not use divs as buttons or links. this makes the whole thing inaccessible as you are removing semantics that screen reader and other assistive technologies use. 

Couple of suggestions: perhaps CSS selector could be more explicit (to override the default rule).
I would also check that this is not an outline that is showing.
In case all of that fails, I would try resetting appearance property to none (https://developer.mozilla.org/en/docs/Web/CSS/-moz-appearance) and styling it the way we would want it to be.

Comment 18

[Fischer [:Fischer]]

Per the discussion with UX: verdi on the meeting, we can go for the given default grey button border in the high-contrast mode. 

About the black button border, currently haven't found a css way for it. Not sure if a grey border is to distinguish button from other elements. Anyway we could investigate this in another bug. For now better to have the onboarding tours handle the high-contrast mode.

Comment 19

[Fischer [:Fischer]]

Attachment: Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

This patch makes the mininmum and the important elements and images visible in the high-constrast mode, including
- making button have border (using the given default grey border in the high-constrast mode)
- making the upper-left overlay button's fox image visible
- making the images of the X close buttons on the notification bar and on the overlay visible
- making that the navigation item of the current tour on the overlay's right side would have outline
- making the hovered navigation item on the overlay's right side would have outline
- making sure using <button> element for buttons for a better semantic
- changing #onboarding-overlay-icon to #onboarding-overlay-button for a better semantic

Review commit: https://reviewboard.mozilla.org/r/158184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158184/

Comment 20

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #19)
> Created attachment 8887356 [details]
> Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display
> mode,
> 
> This patch makes the mininmum and the important elements and images visible
> in the high-constrast mode, including
> - making button have border (using the given default grey border in the
> high-constrast mode)
> - making the upper-left overlay button's fox image visible
> - making the images of the X close buttons on the notification bar and on
> the overlay visible
> - making that the navigation item of the current tour on the overlay's right
> side would have outline
> - making the hovered navigation item on the overlay's right side would have
> outline
> - making sure using <button> element for buttons for a better semantic
> - changing #onboarding-overlay-icon to #onboarding-overlay-button for a
> better semantic
> 
> Review commit: https://reviewboard.mozilla.org/r/158184/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/158184/
Hi Mossop,

Please see 
- attachment 8884155 [details]: handle_high_contrast_mode_when_enabling_high_contrast_mode_1.png
- attachment 8884156 [details]: handle_high_contrast_mode_when_enabling_high_contrast_mode_2.png
- attachment 8884159 [details]: handle_high_contrast_mode.gif
for the ui.

thank you.

Comment 21

[Fischer [:Fischer]]

Comment on attachment 8887356 [details]
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158184/diff/1-2/

Comment 22

[Dave Townsend [:mossop]]

Comment on attachment 8887356 [details]
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

https://reviewboard.mozilla.org/r/158184/#review164372

Comment 23

[[e-mail]]

Comment on attachment 8887356 [details]
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

https://reviewboard.mozilla.org/r/158184/#review164520

::: browser/extensions/onboarding/content/onboarding.js:708
(Diff revision 2)
> -    img.id = "onboarding-overlay-icon";
> -    return img;
> +    button.id = "onboarding-overlay-button";
> +    let img = this._window.document.createElement("img");
> +    img.id = "onboarding-overlay-button-icon";
> +    img.src = "resource://onboarding/img/overlay-icon.svg";
> +    button.appendChild(img);
> +    return button;

please rebase since rex has landed the welcome message for fox icon

Comment 24

[Fischer [:Fischer]]

Need to rebase on and check-in after the bug 1380963 and 1357017.

Comment 25

[Fischer [:Fischer]]

Comment on attachment 8887356 [details]
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158184/diff/2-3/

Comment 26

[Fischer [:Fischer]]

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

Comment 27

[Fischer [:Fischer]]

Remove checkin-needed because the bug 1380963 is not yet checked in

Comment 28

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 927658d278dc -d 4df8bfcb4e9f: rebasing 408171:927658d278dc "Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode, r=mossop" (tip)
merging browser/extensions/onboarding/content/onboarding.css
merging browser/extensions/onboarding/content/onboarding.js
warning: conflicts while merging browser/extensions/onboarding/content/onboarding.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 29

[Fischer [:Fischer]]

Checkin-needed again because the bug 1380963 was checked in

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9b1c60ebd4f
Should adapt the oboarding UI to the hight-contrast display mode, r=mossop

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 32

[Justin [:JW_SoftvisionQA]]

I have verified this bug on today's nightly.