The default bug view has changed. See this FAQ.

don't cache is form fill enabled pref

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tbsaunde, Assigned: capella)

Tracking

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
remove the code in accessible/src/base/nsAccessNode.cpp/h that has to do with gIsFormFillEnabled and in accessible/src/html/HTMLFormControlAccessible.cpp just check preferences::GetBoolPref() directly.
(Assignee)

Comment 1

5 years ago
Created attachment 620564 [details] [diff] [review]
Patch (v1)

Initial patch attempt ... builds and tests locally.
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #620564 - Flags: feedback?(trev.saunders)
(Reporter)

Comment 2

5 years ago
Comment on attachment 620564 [details] [diff] [review]
Patch (v1)

>+#include "nsIPrefService.h"
> #include "nsISelectionController.h"
> #include "jsapi.h"
>+  if (mParent) {
>+    bool isFormFillEnabled = false;
>+    nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
>+    if (prefBranch)
>+      prefBranch->GetBoolPref("browser.formfill.enable", &isFormFillEnabled);

since your touching this exact code already use mozilla::preferneces please, if nothing else it'll make the changes here easier to be sure are correct.
Attachment #620564 - Flags: feedback?(trev.saunders)
(Assignee)

Updated

5 years ago
Depends on: 750283
(Assignee)

Comment 3

5 years ago
Created attachment 620687 [details] [diff] [review]
Patch (v2)

Cool tip! Much cleaner code.
Attachment #620564 - Attachment is obsolete: true
Attachment #620687 - Flags: feedback?(trev.saunders)
(Assignee)

Updated

5 years ago
Blocks: 750295
(Reporter)

Comment 4

5 years ago
Comment on attachment 620687 [details] [diff] [review]
Patch (v2)

>   // is unattached from tree then we don't care.
>-  if (mParent && gIsFormFillEnabled) {
>+  if (mParent && mozilla::Preferences::GetBool("browser.formfill.enable", true)) {

add using namespace mozilla then you can just do preferences::

also keep the defautl as false.

>     // Check to see if autocompletion is allowed on this input. We don't expose
>     // it for password fields even though the entire password can be remembered
>     // for a page if the user asks it to be. However, the kind of autocomplete
>     // we're talking here is based on what the user types, where a popup of
>     // possible choices comes up.
>     nsAutoString autocomplete;
>     mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::autocomplete,
>                       autocomplete);
Attachment #620687 - Flags: feedback?(trev.saunders) → feedback+

Comment 5

5 years ago
Mark, are you going to file new patch addressing comments or should I fix them before landing?
(Assignee)

Comment 6

5 years ago
I'll fix this one .... I'm working you hard enough already   :)

Comment 7

5 years ago
(In reply to Mark Capella [:capella] from comment #6)
> I'll fix this one .... I'm working you hard enough already   :)

thank you. and it'd be great if you get an access to land your patches yourself. it'll be a big help also :)
(Assignee)

Comment 8

5 years ago
Created attachment 621666 [details] [diff] [review]
Patch (v3)

Final nits addressed and re-tested ...
Attachment #620687 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #621666 - Flags: review?(surkov.alexander)

Updated

5 years ago
Attachment #621666 - Attachment is patch: true

Comment 9

5 years ago
Comment on attachment 621666 [details] [diff] [review]
Patch (v3)

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

::: accessible/src/html/HTMLFormControlAccessible.cpp
@@ +498,5 @@
>      return state | states::SUPPORTS_AUTOCOMPLETION;
>  
>    // No parent can mean a fake widget created for XUL textbox. If accessible
>    // is unattached from tree then we don't care.
> +  if (mParent && Preferences::GetBool("browser.formfill.enable", false)) {

you don't need to pass false since it's value of default argument, I'll fix it
Attachment #621666 - Flags: review?(surkov.alexander) → review+

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/87bc642d7328
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/87bc642d7328
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.