[Messages] Message Options appear alongside Keyboard; blocks view of text entry

VERIFIED FIXED in 2.2 S3 (9jan)

Status

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: onelson, Assigned: julienw)

Tracking

({regression})

unspecified
2.2 S3 (9jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified)

Details

(Whiteboard: [2.2-Daily-Testing][sms-sprint-2.2S3], )

Attachments

(2 attachments)

Reporter

Description

5 years ago
Description:
When a user holds tap on a message/attachment, they are given the UI for Message Options. If they keyboard is already present on screen, the Options UI will attempt to hold whatever screen space it may occupy outside of the keyboard. This blocks the user from observing text entry until the Options Menu is dismissed, and presents minor difficulty in selecting an option from the newer menu.
   
Repro Steps:
1) Update a Flame device to BuildID: 20141217040204
2) Open Messages App.
3) Open an existing thread.
4) Tap in Text Field to bring up keyboard.
5) Hold tap on text/image to bring up Message Options.
6) Observe UI.
  
Actual:
Message Options appears alongside Keyboard, obscuring text entry from keyboard.
  
Expected: 
Opening Message Options dismisses keyboard.
  
Environmental Variables:
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141217040204
Gaia: d22dfece04fc00457e8369c660c11f945b088d2f
Gecko: cb8ad2251c09
Gonk: e5c6b275d77ca95fb0f2051c3d2242e6e0d0e442
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

  
Repro frequency: 5/5
See attached: 
video- http://youtu.be/w98E-GKXBiM
Reporter

Comment 1

5 years ago
Issue had been observed before and was resolved fixed (bug 948356) in v1.3
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Whiteboard: [2.2-Daily-Testing]
Assignee

Comment 2

5 years ago
Likely someone removed either the tabindex=-1 or the focus() call. Can we please gave a regression window?
blocking-b2g: --- → 2.2?
triage: regression, and it looks broken.
blocking-b2g: 2.2? → 2.2+
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Confirmed regression; 2.1 is unaffected. Working on window now.
QA Contact: pcheng
b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20141015070318
Gaia: a18559e844b6aec946309a3bee6a32ca06ab4649
Gecko: fe4916e0e9df
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20141015102818
Gaia: 93fb5070782a549977cf8c485c5d93de54ec657e
Gecko: 069db53e0f68
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: 93fb5070782a549977cf8c485c5d93de54ec657e
Gecko: fe4916e0e9df

First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: a18559e844b6aec946309a3bee6a32ca06ab4649
Gecko: 069db53e0f68

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/a18559e844b6aec946309a3bee6a32ca06ab4649...93fb5070782a549977cf8c485c5d93de54ec657e

Caused by Bug 1078295.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Caused by Bug 1078295 - Chris can you take a look please?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado) → needinfo?(chrislord.net)
Assignee

Comment 7

5 years ago
Chris, tell us if you can't take this bug, we can take it in the SMS team.
Assignee

Comment 8

5 years ago
Looks like not, let me try to fix this :)
Assignee: nobody → felash
Flags: needinfo?(chrislord.net)
Assignee

Comment 9

5 years ago
Posted file github PR
Attachment #8540815 - Flags: review?(schung)
Comment on attachment 8540815 [details] [review]
github PR

lgtm, but maybe we can simply call blur while showing the menu. Do you think it's necessary to focus the form for other reason like a11y?
Attachment #8540815 - Flags: review?(schung) → review+
Assignee

Comment 11

5 years ago
Yes, usually we need to focus the dialog when it's displayed so that the screenreader can start reading this element. I don't know how Firefox OS' screen reader work though, but I suspect it's the same.

Hey Eitan, can you please confirm?
Flags: needinfo?(eitan)
This should be fine. We do the right thing when a dialog appears and set the cursor on the first item (usually the header). If you want the cursor somewhere specific using focus() will work too. But in this case, I think the header is fine.

p.s.
I was not able to actually test this, since we don't support long tap yet :p
Flags: needinfo?(eitan)
Hey Julien,

I see similar issue for the following case too (after patch for bug 1105857 is landed):

* Open Composer and enter some text into message field;
* Tap on Home button and return back to the Homescreen;
* Receive new message and tap on notification;

Expected result: there should be displayed confirmation dialog (asking if you want to discard draft) only;

Actual result: confirmation dialog is displayed alongside with the keyboard.

Please see new "alert" and "confirm" in Utils.
Assignee

Comment 14

5 years ago
Comment on attachment 8540815 [details] [review]
github PR

Hey Oleg, can you please double check that this looks right?

I thought of a follow-up: save document.activeElement so that we can focus it after hiding. I started doing it and then I had an issue with unit testing it (activeElement is always document.body in unit tests, I couldn't find why), and I thought it would be better in a follow-up than in a 2.2+ bug. I'll file a separate bug for this.
Attachment #8540815 - Flags: review?(azasypkin)
Comment on attachment 8540815 [details] [review]
github PR

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #14)
> Comment on attachment 8540815 [details] [review]
> github PR
> 
> Hey Oleg, can you please double check that this looks right?
> 

Looks good! Just one minor nit and one question at Github.

> I thought of a follow-up: save document.activeElement so that we can focus
> it after hiding. I started doing it and then I had an issue with unit
> testing it (activeElement is always document.body in unit tests, I couldn't
> find why), and I thought it would be better in a follow-up than in a 2.2+
> bug. I'll file a separate bug for this.

Yeah, I agree, it's a perfect fit for follow-up :)
Attachment #8540815 - Flags: review?(azasypkin) → review+
Assignee

Updated

5 years ago
Blocks: 1116772
Assignee

Comment 16

5 years ago
master: 334532c0fcb809ac1c8bcd64a8eb9357967acf36
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Assignee

Updated

5 years ago
Blocks: 1078295
Whiteboard: [2.2-Daily-Testing] → [2.2-Daily-Testing][sms-sprint-2.2S3]
Target Milestone: --- → 2.2 S3 (9jan)
This bug has been verified as "pass" on latest build of Flame 2.2 & Nexus5 2.2 by STR in comment 0.

Actually result: Opening Message Options dismisses keyboard.
Reproduce rate: 0/10
See attachment: Flame_V2.2_Verify1.3gp

Device: Flame 2.2 (pass):
Build ID               20150713002503
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150713.041147
Firmware Date          Mon Jul 13 04:11:59 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 2.2(pass)
Build ID               20150713162504
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a5db6d9850f6
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150713.200629
Firmware Date          Mon Jul 13 20:06:47 EDT 2015
Bootloader             HHZ12f
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.