Closed Bug 1377439 Opened 7 years ago Closed 7 years ago

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

Categories

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

Product:
Firefox
Component:
New Tab Page
Version:
55 Branch
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: Fischer, Assigned: Fischer)

References

Details

(Whiteboard: [photon-onboarding])

Attachments

(10 files)

not_handle_high_contrast_mode_1.png
89.10 KB, image/png
Details
not_handle_high_contrast_mode_2.png
143.37 KB, image/png
Details
handle_high_contrast_mode_when_disabling_high_contrast_mode_1.png
214.83 KB, image/png
Details
handle_high_contrast_mode_when_disabling_high_contrast_mode_2.png
152.52 KB, image/png
Details
handle_high_contrast_mode_when_enabling_high_contrast_mode_1.png
89.99 KB, image/png
Details
handle_high_contrast_mode_when_enabling_high_contrast_mode_2.png
143.99 KB, image/png
Details
handle_high_contrast_mode.gif
3.10 MB, image/gif
Details
button_grey_outline_under_hight_contrast_mode.png
217.26 KB, image/png
Details
div_as_button_black_outline_under_hight_contrast_mode.png
172.63 KB, image/png
Details
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,
59 bytes, text/x-review-board-request
mossop
: review+
Details 
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
Assignee: nobody → fliu
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [photon-onboarding]
Target Milestone: --- → Firefox 57
Flags: qe-verify+
QA Contact: jwilliams
Summary: Should not use background-image to display image → Should adapt the oboarding UI to the hight-contrast display mode

    
    

    
  
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.
Flags: needinfo?(mverdi)

    
  

        Attachment #8884159 -
        Flags: ui-review?(mverdi)

    
    

    
  
Are you sure this is not 56?
Target Milestone: Firefox 57 → Firefox 56

    
    

    
  
Ni for above question.
Flags: needinfo?(fliu)
Priority: P2 → P1

    
    

    
  
(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.
Flags: needinfo?(fliu)

    
    

    
  
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?
Flags: needinfo?(mverdi) → needinfo?(yzenevich)

    
    

    
  
(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/
Flags: needinfo?(yzenevich)

    
    

    
  
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.

    
    

    
  
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.

    
    

    
  
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.

    
  
Blocks: 1377273

    
    

    
      
  


  

    
    

    
  
(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 on attachment 8887356 [details]
Bug 1377439 - Should adapt the oboarding UI to the hight-contrast display mode,

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

        Attachment #8887356 -
        Flags: review?(dtownsend) → review+

    
    

    
  
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

    
    

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

    
    

    
      
  

    
    

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

    
    

    
  
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)

    
    

    
  
Checkin-needed again because the bug 1380963 was checked in
Keywords: checkin-needed

    
    

    
  
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e9b1c60ebd4f
Should adapt the oboarding UI to the hight-contrast display mode, r=mossop
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/e9b1c60ebd4f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox56:
          --- → fixed
Resolution: --- → FIXED

    
    

    
  
I have verified this bug on today's nightly.
Status: RESOLVED → VERIFIED

        Attachment #8884159 -
        Flags: ui-review?(mverdi)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: