Last Comment Bug 834613 - Field zoom is not toggleable (formhelper.* prefs need hooking up)
: Field zoom is not toggleable (formhelper.* prefs need hooking up)
Status: RESOLVED FIXED
[parity-xul]
:
Product: Firefox for Android
Classification: Client Software
Component: Graphics, Panning and Zooming (show other bugs)
: 21 Branch
: ARM Android
: -- normal with 1 vote (vote)
: Firefox 25
Assigned To: Botond Ballo [:botond] (C++ standards meeting Jun 20-25)
:
Mentors:
: 872266 (view as bug list)
Depends on:
Blocks: 725018
  Show dependency treegraph
 
Reported: 2013-01-24 22:55 PST by vinceying113
Modified: 2013-07-16 13:30 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.67 KB, patch)
2013-07-12 14:25 PDT, Botond Ballo [:botond] (C++ standards meeting Jun 20-25)
bugmail.mozilla: review+
Details | Diff | Review
updated patch (3.17 KB, patch)
2013-07-15 11:16 PDT, Botond Ballo [:botond] (C++ standards meeting Jun 20-25)
bugmail.mozilla: review+
Details | Diff | Review

Description vinceying113 2013-01-24 22:55:03 PST
User Agent: Mozilla/5.0 (Android; Linux i686; rv:21.0) Gecko/21.0 Firefox/21.0
Build ID: 20130124162801

Steps to reproduce:

Tried to disable form zooming with formhelper.zoom in about:config.  The zoom feature makes filling out long forms tedious due to contant zooming in and out.


Actual results:

Nothing.


Expected results:

Should have disabled the formhelper.zoom functionality.
Comment 1 Aaron Train [:aaronmt] 2013-01-28 08:43:12 PST
The pref is 'formhelper.autozoom' but I believe that is not what you want. Are you referring to the recently added functionality in bug 725018? I don't believe the patches added a toggle state preference.
Comment 2 Aaron Train [:aaronmt] 2013-01-28 08:46:44 PST
(In reply to vinceying113 from comment #0)
> Tried to disable form zooming with formhelper.zoom in about:config.  The
> zoom feature makes filling out long forms tedious due to contant zooming in
> and out.

If you switch between field, why would this cause a zoom out?

Do you have a URL you've been seeing this on?
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2013-01-28 09:37:38 PST
We should probably make the behaviour from bug 725018 depend on these prefs. The prefs look like they were used in XUL fennec.
Comment 4 vinceying113 2013-01-28 09:48:01 PST
Aaron, the reason I want to disable the zoom is Firefox zooms all the way in to the field - so descriptive text next to the field is not visible.  So, on a form with lots of fields, I have to zoom out to view the descriptor on the next field, then click on the field which zooms all the way in, and so on.  Thanks.
Comment 5 Aaron Train [:aaronmt] 2013-01-28 09:51:40 PST
I see, thanks; I was thinking that you were jumping directly between fields and it would first zoom out.
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2013-05-21 13:48:14 PDT
*** Bug 872266 has been marked as a duplicate of this bug. ***
Comment 7 Aaron Train [:aaronmt] 2013-05-27 11:59:09 PDT
This worth tracking for recent browser preference changes and additions?
Comment 8 Christopher Smith 2013-05-27 12:06:12 PDT
Probably.  IMO, the autozoom feature is broken (jumping to max zoom on tiny form fields, where you can't even see next one-line field without scrolling), and I figure I'm probably not the only one with that opinion.  The WebOS browser had a zoom disable, and I turned it off even though that zoom wasn't as frustrating as Firefox's.
Comment 9 Wesley Johnston (:wesj) 2013-05-27 12:34:02 PDT
Can you file a bug with a link to an example? I'd like to try and fix any issues people find (but I'm not opposed to making this pref work either).
Comment 10 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-12 14:25:23 PDT
Created attachment 774912 [details] [diff] [review]
patch

The attached patch hooks up formhelper.mode, and removes the other formhelper.* options (autozoom, autozoom.caret, and restore). The OP's desire (disabling auto-zooming) can now be accomplished by setting formhelper.mode to 0.

Here is the rationale, based on my understanding of the code (I am new to this code so please forgive me and correct me if I've gotten something wrong):

  - The rationale for removing formhelper.autozoom.caret and formhelper.restore 
    is that the underlying functionality is not implemented in native fennec. 
    If it's later implemented, they can be added again.

  - The rationale for removing formhelper.autozoom is that in native fennec, 
    the formhelper doesn't currently do anything besides zooming, and so
    formhelper.mode and formhelper.autozoom would mean the same thing
    (except that formhelper.mode allows discriminating between tablets and
    phones, a flexibility that I think is desirable).
Comment 11 Christopher Smith 2013-07-12 14:32:43 PDT
I don't know enough about the codebase to review the patch, but the described solution sounds practical as long as the prefs' meanings are consistent across whatever platforms they're implemented on (I know I carry preference bundles with me across machines).  The doc for Form Assistant isn't clear but suggests that formhelper.mode should also control the autocomplete popup, which *is* particularly helpful on mobile, and it seems that autozoom was intended to be (is?) controlled separately.  (Also, of course, I found the pref by searching about:config for "zoom"!  "formhelper.mode" is not at all obvious and should have an option in the UI if that's where this control lives.)
Comment 12 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-12 14:38:46 PDT
(In reply to Christopher Smith from comment #11)
> The doc for Form Assistant isn't clear

Where can I find this doc?
Comment 13 Christopher Smith 2013-07-12 14:41:56 PDT
https://wiki.mozilla.org/Mobile/Fennec/FormAssistant is the only Google result for "formhelper.mode" and appears to be the only description of the preference's settings.
Comment 14 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-12 15:03:00 PDT
(In reply to Christopher Smith from comment #11)
> The doc for Form Assistant
> isn't clear but suggests that formhelper.mode should also control the
> autocomplete popup, which *is* particularly helpful on mobile, and it seems
> that autozoom was intended to be (is?) controlled separately.

If I'm reading the code right, xul fennec used auto-complete regardless of formhelper.mode (and current fennec certainly does). Do you think that should change?

Either way, I will update the wiki once we settle on a solution.
Comment 15 Christopher Smith 2013-07-12 15:05:36 PDT
My experience with autocomplete on Android has been... erratic.  I think the option to disable it should be a privacy setting, and I don't know enough to suggest where in the namespace it should go.
Comment 16 Kartikaya Gupta (email:kats@mozilla.com) 2013-07-12 19:48:53 PDT
Comment on attachment 774912 [details] [diff] [review]
patch

Review of attachment 774912 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Please update the commit message though, so that it reflects what the patch is doing rather than just copying the bug summary.
Comment 17 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-15 11:16:51 PDT
Created attachment 775806 [details] [diff] [review]
updated patch

Upon a more careful reading of the code, the current form helper performs two tasks that deserve to be differentiated: scrolling to bring the focused input element into view, and zooming in on it.

The updated patch:

  - Modifies the default behaviour (formhelper.mode = 2) so that 
    scrolling and zooming happens on phones and only srolling happens 
    on tablets. Setting formhelper.mode = 1 enables zooming on
    tablets as well. Setting formhelper.mode = 0 disables both
    scrolling and zooming everywhere.

  - Restore the formhelper.autozoom preferenece (defaults to true),
    which can be set to false to disable zooming for all devices.
Comment 18 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-15 12:05:39 PDT
I updated https://wiki.mozilla.org/Mobile/Fennec/FormAssistant to reflect the changes in the updated patch.
Comment 19 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-15 12:08:40 PDT
Try results for the updated patch: https://tbpl.mozilla.org/?tree=Try&rev=6aa969de4c85
Comment 20 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-15 12:11:48 PDT
(In reply to Christopher Smith from comment #11)
> (Also, of
> course, I found the pref by searching about:config for "zoom"! 
> "formhelper.mode" is not at all obvious and should have an option in the UI
> if that's where this control lives.)

This concern is addressed by restoring the formhelper.autozoom preference in the updated patch.
Comment 21 Botond Ballo [:botond] (C++ standards meeting Jun 20-25) 2013-07-15 12:17:46 PDT
(In reply to Christopher Smith from comment #15)
> My experience with autocomplete on Android has been... erratic.  I think the
> option to disable it should be a privacy setting, and I don't know enough to
> suggest where in the namespace it should go.

I filed bug 893979 for this.
Comment 22 Ryan VanderMeulen [:RyanVM] 2013-07-16 06:15:30 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e3d601a9ab2
Comment 23 Ryan VanderMeulen [:RyanVM] 2013-07-16 13:30:13 PDT
https://hg.mozilla.org/mozilla-central/rev/3e3d601a9ab2

Note You need to log in before you can comment on or make changes to this bug.