Bug 254592 - Updated "Find As You Type" (FAYT) to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=254592, specifically: r=mikedeboer
46 bytes, text/x-phabricator-request
|Details | Review|
If Use Find as you type in options is off only userpref accessibility.typeaheadfind is set to false userpref accessibility.typeaheadfind.autostart is left default (=true) For people using chats and fora it's extremely annoying if you use ' or / and the autostart starts the find toolbar and moves focus to that input. expected: if accessibility.typeaheadfind is set false accessibility.typeaheadfind.autostart should respect that and regarless of its value not autostart (respect accessibility.typeaheadfind false).
This bug is fixed with the patch in bug 251073
Depends on: 251073
OS: Windows 2000 → All
Hardware: PC → All
Summary: Options , Find as you type = off , should respect typeaheadfind and not allow autostart → Find as you type = off pref, should respect typeaheadfind and not allow autostart
*** Bug 264814 has been marked as a duplicate of this bug. ***
No longer depends on: 251073
(In reply to comment #1) > This bug is fixed with the patch in bug 251073 so typing a / or a ' on a page (not in an input) causing the findbar to popup is designed behaviour ? I though this would be disabled, only leaving you the option to press F3 to activate it ?
(In reply to comment #3) > (In reply to comment #1) > > This bug is fixed with the patch in bug 251073 > > so typing a / or a ' on a page (not in an input) causing the findbar to popup is > designed behaviour ? > I though this would be disabled, only leaving you the option to press F3 to > activate it ? The final patch to fic bug 251073 was not that patch that included the fix for this bug. I am working on a new patch to fix the remainder of the preference issues including this one.
this bug is fixed with the patch in bug 265915
*** Bug 290625 has been marked as a duplicate of this bug. ***
In v1.06 on MacOS X (10.3.9), the preference "Begin finding as you begin typing" controls the *autostart* mechanism of typeaheadfind. However, the checkbox will modify accessibility.typeaheadfind, *NOT* accessibility.typeaheadfind.autostart. In addition, it appears that accessibility.typeaheadfind.autostart is completely ignored (true/false make no difference in behavior), and accessibility.typeaheadfind controls only the autostart mechanism. There appears to be no way to totally disable typeaheadfind because of this.
*** Bug 321418 has been marked as a duplicate of this bug. ***
In v18.104.22.168 for MacOS X, the bug I described in comment #8 is still present... the two typeaheadfind preferences are not behaving properly, as outlined in that comment.
The same problem occurs in Firefox 2.0 beta 1 (20060710) on Windows
Sorry, the problem in the latest Windows beta is that any typing outside a text field will activate type-ahead find, even if accessibility.typeaheadfind.autostart is false. The MozillaZine knowledge base links to this bug for that problem. :-(
*** Bug 348187 has been marked as a duplicate of this bug. ***
The history of the 'autostart' pref wasn't very clear to me, until I dived a little deeper into CVS and bugzilla history. So this was implemented in bug 176296, which was back when _any_ keystroke would initiate a find-in-page and "'" or "/" were used to switch between text-mode and links-only-mode. Today these keys are used as activators for Quick Find (showing a minimally marked up findbar), and the 'autostart' pref is not used anymore. Additionally, the 'accessibility.typeaheadfind.autostart' and 'accessibility.typeaheadfind.enabletimeout' are not used anymore. Therefore it seems prudent to ensure that 1. 'accessibility.typeaheadfind.autostart = false' disables Quick Find for those who wish it, 2. 'accessibility.typeaheadfind.enabletimeout' is removed from all.js. Upping priority, since this bug is now actionable.
Priority: P5 → P2
- The accessibility.typeaheadfind preference turns off ALL FAYT functions when set to 'false' - The keystroke-interpretation code in FindBarChild.jsm and findbar.xml now respects both the accessibility.typeaheadfind.manual and typeaheadfind.autostart preferences - Documents of any type can manually inhibit FAYT with the "disablefastfind" document element attribute (previously restricted to the chrome: and about: protocols) - Corrected & extended the pertinent unit tests (browser_fastfind* test modules)
Uploaded patch at https://phabricator.services.mozilla.com/D3404 to simplify/correct this to what I think is more intuitive logic: * Setting accessibility.typeaheadfind to 'false' inhibits ALL fast-find triggers * When accessibility.typeaheadfind and accessibility.typeaheadfind.manual are both true, typing a / or ' will trigger the fast-find dialog * When accessibility.typeaheadfind and accessibility.typeaheadfind.autostart are both true, typing any character except a space will trigger the fast-find dialog * Note that when all three preference variables are true, the manual trigger characters (/ or ') will trigger but not populate the dialog, i.e. you would have to type ' or / twice to begin a search string with those characters. When autostart is true but the manual trigger is not, ' and / are treated exactly like any other keys. This was poorly defined before and this convention seems preferable (less confusing to those used to the manual shortcuts). If accepted, http://kb.mozillazine.org/Accessibility.typeaheadfind should be updated to reflect the new behavior. Even if the patch is not accepted, someone should correct the relevant variable names in the new toolkit/actors/FindBarChild.jsm to match toolkit/content/widgets/findbar.xml (leading underscores in _findAsYouType, _manualFAYT)--- right now this typo breaks most FAYT behavior.
Comment on attachment 9000194 [details] Bug 254592 - Updated "Find As You Type" (FAYT) to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=254592, specifically: r=mikedeboer Kris Maglione [:kmag] has approved the revision.
Attachment #9000194 - Flags: review+
7 months ago
Assignee: nobody → spillner
Hi! When trying to land this on autoland trough phabricator(lando) I received the following error: Error: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmpjkVhcf\npatching file toolkit/content/widgets/findbar.xml\nHunk #2 FAILED at 327\n1 out of 16 hunks FAILED -- saving rejects to file toolkit/content/widgets/findbar.xml.rej\nabort: patch failed to apply', '')
Please see Comment 21. Thanks!
Hi spillner The commit message for the patch needs fixing. You can use this as guideline: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities
Reset to original commit message. I've also submitted the following text to the MozillaZine knowledge base thread as a correction for http://kb.mozillazine.org/Accessibility.typeaheadfind: "When accessibility.typeaheadfind is set to false, all Find As You Type functionality is disabled. When both accessibility.typeaheadfind and accessibility.typeaheadfind.manual are set to true, typing a ' or / will open the Find As You Type window and move the cursor focus there. If accessibility.typeaheadfind and accessibility.typeaheadfind.autostart are both true, typing anything other than a space will open the Find As You Type window. If accessibility.typeaheadfind.timeout is set to a value greater than zero, the Find As You Type window will disappear that many milliseconds after the last keypress."
The issue with the commit message is the first line - you're basically referencing the bug number twice and not clearly summarizing what the patch is actually doing. I would expect it to be something more along the lines of: Bug 254592 - Update "Find As You Type" (FAYT) to respect the accessibility typeaheadfind preferences. r=kmag Or something to that effect. Anyway, that's more in line with our usual conventions for commit messages.
The patch fixes the bug. The next three lines of the commit message describe the new behavior. The tests provide some level of confidence that the browser satisfies that specification. I don't see much value in anything either shorter or longer. Feel free to edit it yourself if I'm missing something.
You need to log in before you can comment on or make changes to this bug.