Closed Bug 641836 Opened 13 years ago Closed 5 years ago

[Regression]The search input should get the focus after loading about:config

Categories

(Firefox for Android Graveyard :: General, defect)

Firefox 5
ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX
Firefox 6

People

(Reporter: anamaria.moldovan, Assigned: vingtetun)

References

()

Details

Attachments

(1 file)

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre Fennec /4.0b6pre 

Device: Motorola Droid 2 (Android 2.2)

Steps to reproduce:
1.Go to about:config via the url bar

Actual results:
The search field is not in focus. One has to tap on the search field to type in something. 

Expected results: 
The search field should get focus, so you can directly type something after about:config has loaded.
The search field gets focus only after reloading the page.
Attached patch PatchSplinter Review
Mark, the patch activate the strict ime checking on device and remove the hack to clear the focus during page load. It also add a method named Util.isKeyboardOpened that map to the ViewableAreaObserver.isKeyboardOpened in both content and chrome process.
Assignee: nobody → 21
Attachment #533622 - Flags: review?(mark.finkle)
Comment on attachment 533622 [details] [diff] [review]
Patch

We can't make the browser binding depend on ViewableAreaObserver
Attachment #533622 - Flags: review?(mark.finkle) → review-
Comment on attachment 533622 [details] [diff] [review]
Patch

/me rubs eyes
Attachment #533622 - Flags: review- → review?(mark.finkle)
Comment on attachment 533622 [details] [diff] [review]
Patch

>diff --git a/mobile/chrome/content/bindings/browser.js b/mobile/chrome/content/bindings/browser.js

>-    // Keep track of hash changes
>-    this.hashChanged = (location == this._lastLocation);
>     this._lastLocation = location;

You can remove _lastLocation too. MXR says it is only used to keep hashChanged updated

>diff --git a/mobile/chrome/content/forms.js b/mobile/chrome/content/forms.js

>+    // hack bug 604351
>+    // if the element is the same editable element and the VKB is closed, reopen it
>+    if (!Util.isKeyboardOpened && aElement instanceof HTMLInputElement && aElement.mozIsTextField(false)) {
>+      aElement.blur();
>+      gFocusManager.setFocus(aElement, Ci.nsIFocusManager.FLAG_NOSCROLL | Ci.nsIFocusManager.FLAG_BYMOUSE);
>+    }

I am not a fan of the sync message used in Util.isKeyboardOpen. This appears to be the only place that uses it. What can we do to _not_ use the sync message?

You should re-arrange the if condition so the sync message is the last test:

      if (aElement instanceof HTMLInputElement && aElement.mozIsTextField(false) && !Util.isKeyboardOpened) {

Now the sync message should only be fired if the other two tests are true

r+, but change the if condition, remove _lastlocation and tell me more about the hack for bug 604351
Attachment #533622 - Flags: review?(mark.finkle) → review+
(In reply to comment #6)
> Comment on attachment 533622 [details] [diff] [review] [review]
> >diff --git a/mobile/chrome/content/forms.js b/mobile/chrome/content/forms.js
> 
> >+    // hack bug 604351
> >+    // if the element is the same editable element and the VKB is closed, reopen it
> >+    if (!Util.isKeyboardOpened && aElement instanceof HTMLInputElement && aElement.mozIsTextField(false)) {
> >+      aElement.blur();
> >+      gFocusManager.setFocus(aElement, Ci.nsIFocusManager.FLAG_NOSCROLL | Ci.nsIFocusManager.FLAG_BYMOUSE);
> >+    }
> 
> I am not a fan of the sync message used in Util.isKeyboardOpen. This appears
> to be the only place that uses it. What can we do to _not_ use the sync
> message?

If needed we can send message from chrome to content each time the state of the VKB is changed. But this probably means more unuseful traffic on the ipc bus for something that is not really useful. I would tend to think that we want to avoid "sync" for things that are done often.

> r+, but change the if condition, remove _lastlocation and tell me more about
> the hack for bug 604351

Sadly if a field already has the focus and receive a mouse click, it's internal IME state won't be updated (the ESM will not inform the IME code of such a change) and so we won't react to that on the Android side. This is why we need to do that. Still not a perfect solution works for most of the cases.
http://hg.mozilla.org/mozilla-central/rev/4eeafccee1f9
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
It seems to works on central:

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110522 Firefox/6.0a1 

Device: HTC Desire Z (Android 2.2)

I have also checked on aurora channel and I can still reproduce this issue. I set the version to Firefox 5 and reopened the bug.

aurora channel:
Mozilla /5.0 (Android;Linux armv7l;rv:5.0a2) Gecko/20110522 Firefox/5.0a2 Fennec/5.0a2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Version: Trunk → Firefox 5
this patch was only pushed to mozilla-central. you won't see it fixed on Aurora
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110523 Firefox/6.0a1 Fennec/6.0a1 

Device: HTC Desire Z (Android 2.2)
Status: RESOLVED → VERIFIED
Target Milestone: --- → Firefox 6
Depends on: 688783
This was backed out because of bug 669995:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eadc362c7929

It can re-land when bug 669995 is fixed (along with any modifications required by the changes in bug 669995).
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Blocks: 690280
Still occurs on the latest Beta build.

--
Firefox 13.0b1 (2012-04-25)
Device: Asus EEE Transformer TF101
OS: Android 4.0.3
Closing all opened bug in a graveyard component
Status: REOPENED → RESOLVED
Closed: 13 years ago5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: