Closed Bug 466770 Opened 16 years ago Closed 14 years ago

fix content autocomplete

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(fennec1.0-)

VERIFIED FIXED
Tracking Status
fennec 1.0- ---

People

(Reporter: Gavin, Assigned: mfinkle)

References

Details

Attachments

(6 files, 2 obsolete files)

Content autocomplete was disabled in bug 466728 because it's pretty broken (the popup was showing up above the canvas, and wasn't taking into account panning/zooming). We should probably fix it somehow to get form autocomplete working again...
Depends on: 461125
tracking-fennec: --- → ?
Flags: wanted-fennec1.0?
tracking-fennec: ? → 1.0-
Flags: wanted-fennec1.0? → wanted-fennec1.0+
Attached patch wip-1Splinter Review
This wip is fully functionnal but need some code clean up.
See the next screenshot to get an idea about the result.
This alternative approach adds a horizontal scroll area below the "prev" and "next" controls. The autofill entries are added as buttons and can be scrolled, similar to the search plugins.

Although this patch does not support it, I want to add the ability to incrementally type in the edit box to filter the entries more using the frecency feature added to the platform for 1.9.2
screenshot of the horizontal approach
Attached patch Patch v0.1 (obsolete) — Splinter Review
Use Mark Finkle approach for the bar UI and directly the FormAutocomplete component.

I don't use the previous result entry parameter for now.
Attachment #420059 - Flags: review?(mark.finkle)
Attached patch patch 2Splinter Review
This patch adds styling and layout created by madhava. It also unbitrots the previous patch.

Vivien, since the last patch was mostly mine, I think it's ok for you to review this one. The changes you added to my first patch looked good.

I needed to add code to hide the autofill when moving to a new element. Other than that, it was working ok.
Assignee: nobody → mark.finkle
Attachment #420059 - Attachment is obsolete: true
Attachment #431453 - Flags: review?(21)
Attachment #420059 - Flags: review?(mark.finkle)
Comment on attachment 431453 [details] [diff] [review]
patch 2

r+ with the correction of the little background the autocomplete element and also with reverting back the event.preventDefault(), event.stopPropagation() changes which were change because of the switch from "keypress" to "keyup" in an other patch
Attachment #431453 - Flags: review?(21) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/14caf091e87e

From IRC, this patch converts the code to only use "keyup" and puts back the event.preventDefault(), event.stopPropagation() changes.

Browser-chrome tests are coming up shortly
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attached patch tests (obsolete) — Splinter Review
Some browser chrome tests
Attachment #431637 - Flags: review?(21)
Flags: in-litmus?
Attached patch tests 2Splinter Review
Moves the call to BrowserUI.closeAutoComplete before the Browser.newTab call. Tests seem to pass now.
Attachment #431637 - Attachment is obsolete: true
Attachment #431675 - Flags: review?(21)
Attachment #431637 - Flags: review?(21)
verified FIXED on builds:
Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2.2pre) Gecko/20100310
Namoroka/3.6.2pre Fennec/1.1a1

and

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre)
Gecko/20100310 Namoroka/3.6.2pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a2pre) Gecko/20100310
Namoroka/3.7a2pre Fennec/1.1a2pre





Litmus testcase added:

https://litmus.mozilla.org/show_test.cgi?id=11586


Follow-up bugs filed:

https://bugzilla.mozilla.org/show_bug.cgi?id=551517
https://bugzilla.mozilla.org/show_bug.cgi?id=551511
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
Ack, let's start using the in-testsuite flag to make sure we get these automated tests into our framework.
Flags: in-testsuite?
(In reply to comment #12)
> Created an attachment (id=431675) [details]
> tests 2
> 
> Moves the call to BrowserUI.closeAutoComplete before the Browser.newTab call.
> Tests seem to pass now.

Sounds like it depends on the computer, these tests still fails on mine with the same error on the waitFor timeout :(

Reverting back the change into browser_FormAssistant.js to use the setTimeout method and using the same trick into browser_form_autocomplete.js should do the trick.
I know that it is a bad behavior because this slow down the tests, but can we land something like that and I will look at that and provide somes tests for bug 547722 to resolve that. It makes sense because this bug is related to the 400ms delay which is the root cause of our failure here.
Comment on attachment 431675 [details] [diff] [review]
tests 2

I guess the patch is bitrotted now
Attachment #431675 - Flags: review?(21)
You need to log in before you can comment on or make changes to this bug.