Closed Bug 609940 Opened 10 years ago Closed 10 years ago

Android only : caret (cursor) can hide underneath the formfill helper

Categories

(Firefox for Android Graveyard :: General, defect, P2)

ARM
Android
defect

Tracking

(fennec2.0+)

RESOLVED WONTFIX
Tracking Status
fennec 2.0+ ---

People

(Reporter: nhirata, Assigned: vingtetun)

References

Details

(Keywords: testcase, Whiteboard: [VKB])

Attachments

(5 files, 3 obsolete files)

Attached image Screenshot
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101105 Firefox/4.0b8pre Fennec/4.0b3pre

1. have the hardware keyboard out
2. go to http://people.mozilla.com/~nhirata/html_tp/formsninput.html
3. scroll down to the text area box
4. click in the text area box and keep hitting return fast until the caret scrolls out of view

Expected: the caret will still be in focus
Actual: the caret hides under the formfill helper.
tracking-fennec: --- → ?
Flags: in-litmus?
Whiteboard: [VKB]
Vivien - another "pan into view" bug? Feel free to dupe this bug
Assignee: nobody → 21
tracking-fennec: ? → 2.0b3+
> 1. have the hardware keyboard out

uh? do you have a G1 or something like this?
If yes, does this bug only happens when the hardware keyboard is out? (Asking that to find the better way to reproduce with the devices I have currently)
I need help to reproduce this bug
We can get someone from qa to help. In the future, try adding the "qawanted" keyword :). This way we can use our Monday triage sessions to find out which bugs need help.
Keywords: qawanted
(In reply to comment #4)
> We can get someone from qa to help. In the future, try adding the "qawanted"
> keyword :). This way we can use our Monday triage sessions to find out which
> bugs need help.

I will use the flag next time, thanks for the tip :)
tracking-fennec: 2.0b3+ → ?
tracking-fennec: ? → 2.0+
Attached file testcase
I can reproduce this, just by tapping on the textarea in this testcase.

I've seen this bug occuring too, where I'm working with virtual keyboards.
Hmm, on the Android phone, the bug doesn't show that easily, although I can see it directly on Windows, that way.
Attached file testcase2
Ok, this testcase reproduces it for me on windows, the N90 and the Droid.
Steps to reproduce:
- Visit testcase2, scroll down to textarea 4
- Tap on textarea 4

Expected result:
- The caret should be completely in view

Actual result:
- The caret is partly or completely buried under the form helper
Let me know, if you can reproduce it with this testcase. If not, I'll ad back the 'qawanted' keyword (and will try come up with a better testcase).
Keywords: qawantedtestcase
Viven - This bug seems similar to some of other formhelper bugs you have worked one.
(In reply to comment #10)
> Viven - This bug seems similar to some of other formhelper bugs you have worked
> one.

I have already a plan that I'm working on, basically the path to show the form helper, dealing with multiple resizes, size changed of the current view, etc... is wrong. I'm redoing it, this is also the first step to fix bug 622342 in my opinion.
Attached patch wip-1 (obsolete) — Splinter Review
First wip, with some paltform bug inside :(
I'll file bug for them
Attached patch Patch (obsolete) — Splinter Review
The patch add some tests but half of them are disabled because of bug 625341 (sick).
Attachment #502837 - Attachment is obsolete: true
Attachment #503498 - Flags: review?(mark.finkle)
Attached patch Patch (obsolete) — Splinter Review
Since bug 625341 is fixed the patch is the same but with more tests enabled.
Attachment #503498 - Attachment is obsolete: true
Attachment #503527 - Flags: review?(mark.finkle)
Attachment #503498 - Flags: review?(mark.finkle)
ARGH! The situation is worse than ever on the N900 with this patch, only a squared part of the screen is enabled for the webpage, the rest is greyed...
Attached patch PatchSplinter Review
The patch resolve the odd issue I as seeing on Maemo and fix bug 622342
Attachment #503527 - Attachment is obsolete: true
Attachment #503850 - Flags: review?(mark.finkle)
Attachment #503527 - Flags: review?(mark.finkle)
Comment on attachment 503850 [details] [diff] [review]
Patch


>diff --git a/chrome/content/forms.js b/chrome/content/forms.js
>       gFocusManager.setFocus(element, Ci.nsIFocusManager.FLAG_NOSCROLL);
>-      sendAsyncMessage("FormAssist:Show", this._getJSON());
>+      // To ensure we get the current caret positionning of the focused
>+      // element we need to delayed a bit the event
>+      this._executeDelayed(function(self) {
>+        sendAsyncMessage("FormAssist:Show", self._getJSON());

nit: Add a line break before the comment

>   _getCaretRect: function _formHelperGetCaretRect() {
>     let element = this.currentElement;
>+    let focusedElement = gFocusManager.getFocusedElementForWindow(content, true, {});
>     if ((element instanceof HTMLTextAreaElement ||
>         (element instanceof HTMLInputElement && element.type == "text")) &&
>-        gFocusManager.focusedElement == element) {
>+        focusedElement == element) {

These multi-line if tests kill me. I guess I am the only one though.

>diff --git a/chrome/tests/head.js b/chrome/tests/head.js

> let chromeRoot = getRootDirectory(gTestPath);
>-messageManager.loadFrameScript(chromeRoot + "remote_head.js", true);
>+let baseURI = "http://mochi.test:8888/browser/mobile/chrome/";
>+messageManager.loadFrameScript(baseURI + "remote_head.js", true);
> messageManager.loadFrameScript("chrome://mochikit/content/tests/SimpleTest/EventUtils.js", true);

This confuses me. I know we changed how to access scripts in the tests. I thought chromeRoot was the right way?

We still use chromeRoot to load JS in browser_vkb.js and browser_forms.js - is this OK?
http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_vkb.js#5
http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_forms.js#2

r+, but what's the answer for the chromeRoot stuff? and fix the nit
Attachment #503850 - Flags: review?(mark.finkle) → review+
(In reply to comment #18)
> >diff --git a/chrome/tests/head.js b/chrome/tests/head.js
> 
> > let chromeRoot = getRootDirectory(gTestPath);
> >-messageManager.loadFrameScript(chromeRoot + "remote_head.js", true);
> >+let baseURI = "http://mochi.test:8888/browser/mobile/chrome/";
> >+messageManager.loadFrameScript(baseURI + "remote_head.js", true);
> > messageManager.loadFrameScript("chrome://mochikit/content/tests/SimpleTest/EventUtils.js", true);
> 
> This confuses me. I know we changed how to access scripts in the tests. I
> thought chromeRoot was the right way?
> 
> We still use chromeRoot to load JS in browser_vkb.js and browser_forms.js - is
> this OK?
> http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_vkb.js#5
> http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_forms.js#2
> 
> r+, but what's the answer for the chromeRoot stuff? and fix the nit

I don't know the gorry details but for there is a security reason that prevent this test to run for me if I use chromeRoot to load remote_head.js.
I think I can move this call internally to browser_formsZoom.js if you think it's better?
How about just adding a comment so we remember that you did it on purpose.
(In reply to comment #20)
> How about just adding a comment so we remember that you did it on purpose.

Will do. I'm a little bit of a scooge with comments :)
http://hg.mozilla.org/mobile-browser/rev/18be310a82a2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
This bug still occurs in :
Mozilla/5.0 (Android; Linux armv71; rv2.0b10pre) Gecko/20110119 Firefox/4.0b10pre Fennec/4.0b4pre
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I found that I got an error when hitting return in the text area box:
Error: browser.getPosition is not a function
Source File: chrome://browser/content/common-ui.js
Priority: -- → P2
We should make sure the error is not happening, but bug 622578 will make the form helper not move content around.

I feel like WONTFIX'ing this bug and moving ahead with bug 622578
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.