Closed Bug 1125045 Opened 9 years ago Closed 9 years ago

A vertical white rectangle shows up in right hand side in APN editor if tapping "Protocol"/"Roaming Protocol" drop down list when keyboard appear.

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, firefox37 wontfix, firefox38 wontfix, firefox39 fixed, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S7 (6mar)
blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: gchang, Assigned: mtseng)

Details

Attachments

(2 files)

### Steps:
1. Open Settings app
2. Navigate to Cellular & Data > SIM 1 in SIM Settings > APN Settings > Data Settings
3. Tap "Add New APN"
4. Scroll down to bottom
5. Long press "default" in APN Type to trigger utility bubble
6. Scroll up a little bit so that "Protocol" & "Roaming Protocol" items can be tapped
7. Tap "Protocol" or "Roaming Protocol", choose "IPv4", and tap "OK" button


### Expected:
1. No vertical white rectangle shows up.

### Actual:
1. A vertical white rectangle shows up.

PS: 
This issue won't happen at tapping "Authentication".

### Reproduce rate
always

### Version:
Gaia-Rev        237008137f6d72b9cad25ff4faff14ff2c40ac63
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/be24dd482a83
Build-ID        20150122162504
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
QA Whiteboard: [textselection]
Blocks: CopyPasteGaia
No longer blocks: CopyPasteLegacy
blocking-b2g: --- → 2.2?
Whiteboard: [systemsfe]
Whiteboard: [systemsfe]
QA Whiteboard: [textselection] → [COM=Text Selection]
Blocks: CopyPasteLegacy
No longer blocks: CopyPasteGaia
Component: Gaia::System → Selection
Product: Firefox OS → Core
I can reproduce this bug even if selection/touch caret is turn off. STR as following.

1. Using edit-prefs.sh to set those preferences to false. ("selectioncaret.enabled", "touchcaret.enabled")
2. Open Settings app
3. Navigate to Cellular & Data > SIM 1 in SIM Settings > APN Settings > Data Settings
4. Tap "Add New APN"
5. Scroll down to bottom
6. Use WebIDE to select "Default".
7. Scroll up a little bit so that "Protocol" & "Roaming Protocol" items can be tapped
8. Tap "Protocol" or "Roaming Protocol", choose "IPv4", and tap "OK" button

Main difference is step 1 and 5. I think this is not related to selection/touch caret feature.
Ahh, the main difference is step 1 and 6, not step 1 and 5.
ni to Howie. Can you triage this bug relevant to text selection.
Flags: needinfo?(hochang)
Hi Arthur, so this is a setting app bug?
Flags: needinfo?(hochang) → needinfo?(arthur.chen)
I found a easier way to reproduce. You don't need to select "Default" word in step 6. Just tap the "Default" word so the keyboard and caret bring up. Then tap "Protocol" and press "OK" button we can reproduce it.
For Web IDE I found the entire html element of settings app was shifted but the displayed position was still (0, 0). Not sure whether this is a graphics issue.
Flags: needinfo?(arthur.chen)
Triage is blocking based on text selection being high priority for 2.2, and copy paste won't work without it. This is also poor UX.
blocking-b2g: 2.2? → 2.2+
After some investigation, I found [1] causes whole page shift left. Based on STR per comment 2, step 6 bring up keyboard. Then in the step 8, <select element> is focused and keyboard dismissed and trigger resize event so that the code in [1] would call <select element>.scrollIntoView(false) and whole page shift left. Kevin, do you have any idea about this? Thanks.

[1]: https://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#1122
Flags: needinfo?(kevingrandon)
Flags: needinfo?(kevingrandon) → needinfo?(kgrandon)
If this is being caused by scrollIntoView I really don't know here =(

We may want to check with some graphics/platform guys?
Flags: needinfo?(kgrandon)
Can this be repro'd without a SIM card?
(In reply to Botond Ballo [:botond] from comment #11)
> Can this be repro'd without a SIM card?

Yes, however, you cannot navigate to SIM manager page without SIM card. But you can use WebIDE to enable SIM manager option and then SIM manager page is able to navigate.

Maybe I could write a simple web page to reproduce this issue. I'll upload once I finish it.
More investigation results...I found there are two problems.

1. In the same page, we have 3 select controls. But only last 2 select controls would cause this issue. So I try to find out what differences between those controls. I found [1] has logic error, this code should be offset.height instead of pos.width. After this change, we don't call <select element>.scrollIntoView so the problem doesn't exist anymore.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#127

2. So the second problem is why calling scrollIntoView on select element in settings would cause whole page shift left. I have no idea what is going on for this problem....
Assignee: nobody → mtseng
Status: NEW → ASSIGNED
Summary: [TextSelection] A vertical white rectangle shows up in right hand side in APN editor if tapping "Protocol"/"Roaming Protocol" drop down list with some text selected → A vertical white rectangle shows up in right hand side in APN editor if tapping "Protocol"/"Roaming Protocol" drop down list when keyboard appear.
I don't think this is related to CopyPaste anymore.
No longer blocks: CopyPasteLegacy
Component: Selection → General
Product: Core → Firefox OS
Comment on attachment 8569697 [details] [diff] [review]
yAxisVisible should compare with offset.height instead of pos.width.

I am unfamiliar with this code. I agree that passing pos.width as the third argument is obviously an error when the method wants a maximum height value, but I don't feel qualified to review the fix.

hg blame says that this code has been in this file for two years and that it was landed by James, so I'm passing the review to him. 

I suspect that Tim would also be a good reviewer for this patch.
Attachment #8569697 - Flags: review?(dflanagan) → review?(jlal)
Comment on attachment 8569697 [details] [diff] [review]
yAxisVisible should compare with offset.height instead of pos.width.

This also looks right to me but I can barely remember the context here... I do know that at the time this landed there where _zero_ tests and I did bunch of manual trial/error by focusing on input elements of various websites so you might need to do play with this a bit.

Passing the r? to tim at worst you have had three people look at this.
Attachment #8569697 - Flags: review?(jlal) → review?(timdream)
Comment on attachment 8569697 [details] [diff] [review]
yAxisVisible should compare with offset.height instead of pos.width.

I am unfamiliar with this code either.
Attachment #8569697 - Flags: review?(timdream) → review?(xyuan)
Attachment #8569697 - Flags: review?(xyuan) → review+
https://hg.mozilla.org/mozilla-central/rev/5a94300d91f7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S7 (6mar)
Comment on attachment 8569697 [details] [diff] [review]
yAxisVisible should compare with offset.height instead of pos.width.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: A vertical space would randomly appear when user press <select> control
Testing completed: On master.
Risk to taking this patch (and alternatives if risky): low risk, just one line fix.
String or UUID changes made by this patch: None
Attachment #8569697 - Flags: approval-mozilla-b2g37?
Attachment #8569697 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
This issue is verified fixed in the latest Nightly Flame 3.0 and 2.2 builds.  

Actual Results: Tapping the Protocol or Roaming Protocol dropdown lists while the keyboard is active does cause a white bar to appear on the far right side.

Environmental Variables:
Device: Flame 3.0 KK (319 MB) (Full Flash)
BuildID: 20150310010227
Gaia: 2fb09da0cb9cefad9c6e40f57533fafda6d12557
Gecko: 6686aacf006f
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2 KK (319 MB) (Full Flash)
BuildID: 20150310002536
Gaia: 166491b92278dc9e648f8d49ab02d9ca00d74421
Gecko: 1cda026f8996
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Text Selection] → [QAnalyst-Triage?][COM=Text Selection]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][COM=Text Selection] → [QAnalyst-Triage+][COM=Text Selection]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: