Closed Bug 977276 Opened 6 years ago Closed 6 years ago

Crash reporter is visible to screen reader when screen is locked.

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.1, tracking-b2g:backlog)

RESOLVED FIXED
feature-b2g 2.1
tracking-b2g backlog

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

(Keywords: access, Whiteboard: [b2ga11y p=1] )

Attachments

(1 file)

It is possible to navigate to the crash screen dialog even when the lockscreen is showing. This should not be the case!
Whiteboard: [b2ga11y 2.0]
Whiteboard: [b2ga11y 2.0] → [b2ga11y p=1]
blocking-b2g: --- → backlog
feature-b2g: --- → 2.1
Re-aligning priorities with 2.1 accessibility goals.
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1]
Assignee: nobody → eitan
Pretty straightforward. The assumption here is that any dialog in dialog-overlay is hidden when the screen is locked.
Attachment #8452701 - Flags: review?(timdream)
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

This is trivial but not sure if this is correct. First, I think the selector is better to be written as |#screen.dialog:not(.locked) #dialog-overlay| here.

https://github.com/eeejay/gaia/blob/bug-977276/apps/system/style/system/system.css#L284-L287

However, I am not sure if it's possible we encounter a situation where we need to show the dialog on top of the lock screen -- your CSS rule will likely break that. Flagging :alive to confirm this.
Attachment #8452701 - Flags: review?(timdream) → feedback?(alive)
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

What Tim said.
Since crash dialog lives in dialog-overlay it's impossible to show it on lockscreen now.

I don't know what we do to b2g crash on boot though. I always see the crash dialog shows without lockscreen if b2g crashed but I am not sure how we do this..
Attachment #8452701 - Flags: feedback?(alive) → feedback+
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

Alive, this patch is about setting visibility hidden to #dialog-overlay, so it will make ALL dialogs inside dialog-overlay (not just the crash dialog) not being able to show on lock screen.

Just want to confirm with you that no dialog in #dialog-overlay will ever appear on top of the lock screen.

PS Crash dialog is the dialog confirming with user on sending crash reports.
Attachment #8452701 - Flags: feedback+ → feedback?(alive)
Dialog overlay includes
* Permission dialog
* App install dialog
* Value selector (will be removed after bug 962434
* System modal dialog
* Emergency callback dialog

Now with current zindex.css it's impossible to display anything under dialog-overlay on lockscreen.
If there's any new UX spec change we could move some of them out of dialog-overlay, but I don't think the patch will cause regression.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #3)
> Comment on attachment 8452701 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505
> 
> This is trivial but not sure if this is correct. First, I think the selector
> is better to be written as |#screen.dialog:not(.locked) #dialog-overlay|
> here.
> 

The crash reporter does not give the screen a class of 'dialog', only subclasses of SystemDialog do that, so changing that rule will not work.
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

After checking again, I am confident anything under #dialog-overlay will not be shown according to z-index.css

Permission has its own z-index and it's out of dialog overlay.

BTW for the long term should be let visibility manager to change UI visibility as well, if we end up having lots of css rules to change visibility..
Attachment #8452701 - Flags: feedback?(alive) → feedback+
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

Reflagging timdream for review after feedback+ from alive.
Attachment #8452701 - Flags: review?(timdream)
Comment on attachment 8452701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505

You did not address my review comment on comment 3 with the |:not()| selector, but I don't think it's something worthy blocking landing of this patch for.
Attachment #8452701 - Flags: review?(timdream) → review+
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #10)
> Comment on attachment 8452701 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21505
> 
> You did not address my review comment on comment 3 with the |:not()|
> selector, but I don't think it's something worthy blocking landing of this
> patch for.

I tried to address that, sorry if it was not clear. The selector won't work for the crash reporter since the screen won't get a class of dialog.
https://github.com/mozilla-b2g/gaia/commit/9c93a08fd3ecc3781c531b46017a563254ce9c90
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.