Closed
Bug 486184
Opened 15 years ago
Closed 15 years ago
make filling in forms easier
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(fennec1.0+)
VERIFIED
FIXED
fennec1.0b5
Tracking | Status | |
---|---|---|
fennec | 1.0+ | --- |
People
(Reporter: madhava, Assigned: vingtetun)
References
Details
(Whiteboard: [fennec l10n])
Attachments
(3 files, 14 obsolete files)
Right now, filling in forms is very difficult. There are a couple of things that would make this easier: - make it easier to focus form fields -- right now, tapping on them, like with links, often doesn't "take" - when focusing a form field, we should zoom in to some smart zoom level around the form field - create a finger-use-appropriate version of a combo box / drop down box
Reporter | ||
Updated•15 years ago
|
tracking-fennec: --- → ?
Reporter | ||
Comment 1•15 years ago
|
||
also - make sure that spatial nav (tabbing from field to field) works
Updated•15 years ago
|
tracking-fennec: ? → 1.0+
Comment 2•15 years ago
|
||
I think this is important enough to start working on it for b3, not waiting for 1.0
Assignee | ||
Comment 3•15 years ago
|
||
For making it easier to focus form field, doesn't it help to focus him on a mousedown (like in Firefox) instead of a complete click ? Maybe we can assume than if a user click on something like an input field or a links and move only for a few pixels he want to click instead of pan? We can make a kind of funnel with condition depending on where append the mouseup. It that's ok I can try to propose a patch a least for trying to make it easier to focus form field and click on something. ( Maybe the things mentionned by Madhava are separated bugs ? )
Comment 4•15 years ago
|
||
(In reply to comment #0) > - create a finger-use-appropriate version of a combo box / drop down box Bug 472426
Comment 5•15 years ago
|
||
(In reply to comment #0) > - make it easier to focus form fields -- right now, tapping on them, like with > links, often doesn't "take" This is working much better now. > - when focusing a form field, we should zoom in to some smart zoom level around > the form field This is something we have not started at all. I'm open to ideas about the "smart" zoom. It does sound like a good idea. How do we guess at a good zoom? 1. Just zoom to the form control, but add bigger margins? (might miss any labels) 2. Look for a parent <form> and zoom to it? (form could be big or missing) 3. Look for an associated <label> and zoom to the label+form? 4. Some combination of #1 and #3?
Reporter | ||
Comment 6•15 years ago
|
||
I think I'm in favour of zooming to the label+field when the field is focused. If a person just wants to get a closer look at the overall form, we should let him or her double-tap within the borders of the form and do our usual zoom to area zooming. By the point a person is actually focusing a field, he or she probably wants to interact with it specifically, so zooming right in makes sense. The tension between that tight zoom and wanting to find the next field is where the design pattern of having "next" and "previous" buttons, as in the iphone's Form Assistant, comes from -- we should probably do something similar.
Updated•15 years ago
|
Assignee: nobody → mark.finkle
Reporter | ||
Comment 7•15 years ago
|
||
Assignee | ||
Comment 8•15 years ago
|
||
- I'll probably move the getLabelForElement method in FormsHelper - label+field zoom still need work. - not sure of want to use an exeption in InputHandler for forms elements. I'm wondering if I should do a xbl binding for all forms element that handle the click? (kill perf?) - Point 3 of madhava's mockup is not implemented yet, I'm not quite sure of the right way to build my implementation for that. * merge select helper + forms helper ? * keep them separate and just do a UI glue ? - some small bugs + css tweaks
Assignee | ||
Comment 9•15 years ago
|
||
Mostly working but: - act as a modal so the user have to click 'Done' before being able to to click anywhere - doesn't allow to use next/previous in form element without a parent form - Opening the urlbar while using the form manager can hide the focus form field
Attachment #391459 -
Attachment is obsolete: true
Assignee | ||
Comment 10•15 years ago
|
||
Assignee | ||
Updated•15 years ago
|
Attachment #391636 -
Attachment is patch: false
Attachment #391636 -
Attachment mime type: text/plain → image/png
Assignee | ||
Comment 11•15 years ago
|
||
Mostly working but: - act as a modal so the user have to click 'Done' before being able to to click anywhere - With the modal mode urlbar act strangely - Need some XXX removals
Attachment #391633 -
Attachment is obsolete: true
Assignee | ||
Comment 12•15 years ago
|
||
- Remove the modal behavior (we have the same bug as 500848), but modal mode prevent us to pan or click or anything, and it's really annoying! - Remove XXX
Attachment #391705 -
Attachment is obsolete: true
Assignee | ||
Comment 13•15 years ago
|
||
Zoom works better now
Attachment #391744 -
Attachment is obsolete: true
Assignee | ||
Comment 15•15 years ago
|
||
Attachment #391868 -
Attachment is obsolete: true
Updated•15 years ago
|
Whiteboard: [fennec l10n]
Comment 16•15 years ago
|
||
Comment on attachment 396930 [details] [diff] [review] Patch v0.1 - updated on trunk >diff -r 9e6d655f6dca chrome/content/browser-ui.js >+ _getAll: function() { This function seems to be called a lot (via _getNext/_getPrevious). Can we cache these results and save _currentIndex rather than _currentElement to avoid that?
Assignee | ||
Comment 17•15 years ago
|
||
(In reply to comment #16) > (From update of attachment 396930 [details] [diff] [review]) > >diff -r 9e6d655f6dca chrome/content/browser-ui.js > > >+ _getAll: function() { > > This function seems to be called a lot (via _getNext/_getPrevious). Can we > cache these results and save _currentIndex rather than _currentElement to avoid > that? Probably, but my test case was the advanced search of bugzilla and the fields change depending on your selection! I'll dive more into it tomorrow morning.
Assignee | ||
Comment 18•15 years ago
|
||
* Updated on trunk * call _getAll only once
Attachment #396930 -
Attachment is obsolete: true
Comment 19•15 years ago
|
||
* Updated to trunk * forms-helper-* form-helper-* (only one form at a time) I tested this and two things jumped out at me: * Next and Previous had a little trouble scrolling into the right spot. The select-container frequently covered the widget * The urlbar positioning was getting messed up. The urlbar would slide left/right as I was panning left/right, which it should never do.
Attachment #401199 -
Attachment is obsolete: true
Assignee | ||
Comment 20•15 years ago
|
||
> * The urlbar positioning was getting messed up. The urlbar would slide > left/right as I was panning left/right, which it should never do. I guess this should be corrected by bug 511224
Assignee | ||
Comment 21•15 years ago
|
||
(In reply to comment #19) > Created an attachment (id=406399) [details] > patch 0.3 (updated to trunk) > > I tested this and two things jumped out at me: > * Next and Previous had a little trouble scrolling into the right spot. The > select-container frequently covered the widget Should works now. > * The urlbar positioning was getting messed up. The urlbar would slide > left/right as I was panning left/right, which it should never do. I have done some little tweaks in the patch about that it shoulds works better now. (and bug 511224 should correct the zoom/urlbar problem we know) !important: With this patch (and all the previous one) the 'select' support in the right panel is broken.
Attachment #406399 -
Attachment is obsolete: true
Assignee | ||
Comment 22•15 years ago
|
||
Correct some minor bugs with invisible elements nested into element with display:none. Also change the border from 0.25mm to 0.1mm. Still not handle select for the right panel though.
Attachment #406589 -
Attachment is obsolete: true
Assignee | ||
Comment 23•15 years ago
|
||
* work with the right panel now * minor code cleanup
Attachment #406656 -
Attachment is obsolete: true
Attachment #407020 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 24•15 years ago
|
||
This should work on trunk now but it still needed a few cleanup. The new zoom mechanism made it works much much smoother! Thanks Stechz!
Attachment #407020 -
Attachment is obsolete: true
Attachment #407020 -
Flags: review?(mark.finkle)
Comment 25•15 years ago
|
||
Comment on attachment 407785 [details] [diff] [review] Patch v0.7 Thoughts: * Working pretty good * Why using the "rootNode.insertBefore" code to handle selects? * I wish we didn't have FormHelper and FormHelper.isOpen used in other places in the code. * Are the additional vboxes and buffer spacers required? I want this patch to land as soon as possible so we can get some back time before beta 5. Let's discuss these issues.
Assignee | ||
Comment 26•15 years ago
|
||
(In reply to comment #25) > (From update of attachment 407785 [details] [diff] [review]) > Thoughts: > * Why using the "rootNode.insertBefore" code to handle selects? The main reason for that is for being able to show just the chrome-select list, not the form helper when you don't need it. For example the menulist for selecting your language. I want to reuse the same xul/js code part when we need the form helper for a list and when we don't need it. This way the only difference between the 2 appproch are some small css tweaks. > * I wish we didn't have FormHelper and FormHelper.isOpen used in other places > in the code. Sure. That's why I have said the patch needs some cleanup. These FormHelper.isOpen flags are ugly. Though, I need a way to limit the zoom limit when we use the form helper because I think that better for the user to have a context of what they are seeing instead of a full zoom. > * Are the additional vboxes and buffer spacers required? We probably need at least one of theses boxes. I'm trying to get a working patch with as few as possible. > I want this patch to land as soon as possible so we can get some back time > before beta 5. Let's discuss these issues.
Assignee | ||
Comment 27•15 years ago
|
||
(In reply to comment #25) > (From update of attachment 407785 [details] [diff] [review]) > * I wish we didn't have FormHelper and FormHelper.isOpen used in other places > in the code. Done! > * Are the additional vboxes and buffer spacers required? The patch use only one vbox and buffer spacer now
Attachment #407785 -
Attachment is obsolete: true
Updated•15 years ago
|
Attachment #408021 -
Flags: review?(mark.finkle)
Updated•15 years ago
|
Attachment #408021 -
Flags: review?(webapps)
Comment 28•15 years ago
|
||
Ben, can you look over the changes to the zoom code?
Assignee | ||
Comment 29•15 years ago
|
||
* Handle the resize of the window
Attachment #408021 -
Attachment is obsolete: true
Attachment #408043 -
Flags: review?(webapps)
Attachment #408043 -
Flags: review?(mark.finkle)
Attachment #408021 -
Flags: review?(webapps)
Attachment #408021 -
Flags: review?(mark.finkle)
Reporter | ||
Comment 30•15 years ago
|
||
Tried it out! Works well. Two things: - can we make the prev/next buttons visually disabled when you're at the beginning or end of the form? They already don't push in when this is the case, but can we make them "greyed" out. - it seems to be slow to move next or previous when the fields involved are multi-select list boxes (I'd land it anyway, though)
Comment 31•15 years ago
|
||
- What do we expect on double tap? Close the form assistant and zoom back out? - If you accidentally click a form element, "Done" doesn't take you back to where you were. Maybe clicking Done should restore the original viewport? - When panning up with form filler on, the url bar becomes visible, even when I'm not at the top of the page.
Comment 32•15 years ago
|
||
Great feedback in comment #30 and comment #31. We could try to get the disabled buttons in on the initial landing. The performance and behavior questions in comment #31 are good for followup decisions and bugs.
Updated•15 years ago
|
Attachment #408043 -
Flags: review?(mark.finkle) → review+
Comment 33•15 years ago
|
||
Comment on attachment 408043 [details] [diff] [review] Patch v0.8 Let's try to disable the buttons when we hit the beginning or end.
Comment 34•15 years ago
|
||
Comment on attachment 408043 [details] [diff] [review] Patch v0.8 >diff -r 9c6a437c3783 chrome/content/browser-ui.js >+ if (labelRect.left < elRect.left) { >+ let width = labelRect.width + elRect.width + (elRect.x - labelRect.x - labelRect.width); >+ return new Rect(labelRect.x, labelRect.y, width, elRect.height).expandToIntegers(); >+ break; >+ } The break is redundant. All of my other code review questions were answered in IRC. My only issue is with the helper spacer, which causes the urlbar to pan onto the screen when it is visible.
Attachment #408043 -
Flags: review?(webapps) → review-
Assignee | ||
Comment 35•15 years ago
|
||
* correct the urlbar pan bug.
Attachment #408135 -
Flags: review?(webapps)
Updated•15 years ago
|
Attachment #408135 -
Flags: review?(webapps) → review+
Updated•15 years ago
|
Attachment #408043 -
Attachment is obsolete: true
Comment 36•15 years ago
|
||
pushed: https://hg.mozilla.org/mobile-browser/rev/f8894cddcdc1 Nice work Vivien. As we get feedback and find bugs, open new bugs. Don't add stuff in this bug.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → B5
Comment 37•15 years ago
|
||
This is up and running even though there's a huge amount of touch points that we need to start testing now...and bugs that need to be filed because this patch did not take care of them and/or brought them up. If you guys are going to bring out new features like this, you better dogfood them. verified FIXED on builds: Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091026 Fennec/1.0b5pre and Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091026 Fennec/1.0b5pre This definitely has to be in our BFTs. Al, do you think we'll need to create a new subgroup for this?
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Comment 38•15 years ago
|
||
This is complex enough that it should have its own section, I think. We're probably going to be adding testcases to this for a while.
Comment 39•15 years ago
|
||
How do you want to address this, Aakash? This isn't a simple one-off testcase for litmus.
Comment 40•15 years ago
|
||
Yeah, I know. I'd suggest re-enabling the subgroup on our BFTs and then creating one or two testcases that test basic functionality for smoketests as well as that BFT.
Comment 41•15 years ago
|
||
litmus testcases 9803, 9804, 9805 were created to regression test this bug.
Flags: in-litmus? → in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•