Closed Bug 256079 Opened 17 years ago Closed 17 years ago

Clicking focused readonly textbox triggers autocomplete

Categories

(Firefox :: Address Bar, defect)

1.0 Branch
x86
Windows XP
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Firefox1.0

People

(Reporter: jruderman, Assigned: bryner)

References

()

Details

(Keywords: fixed-aviary1.0, regression)

Attachments

(2 files, 3 obsolete files)

1. Load 
  data:text/html,<form action="http://www.google.com/"><input name="q" readonly>
2. Click the textbox twice.

Result: Autocomplete widget appears.

You can even use autocomplete to put text into the field, which is invisible but
submitted anyway.
Flags: blocking-aviary1.0?
My build machine is still in a shambles.  What adding this somewhere in
nsFormFillController::MouseClick()?  I think it should compile.

  if (!mFocusedInput)
     return NS_OK;

  nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(target);
  PRBool disabled = PR_FALSE;
  input->GetDisabled(&disabled);
  if (disabled)
    return NS_OK;
Attached patch patch (obsolete) — Splinter Review
I don't know, but isn't it better to add it in nsFormFillController::Focus?
This seems to work for me. No autocomplete anymore for readonly textboxes.
Comment on attachment 156743 [details] [diff] [review]
patch

>+    if (type.EqualsLiteral("text") && (isReadOnly == PR_FALSE) &&

&& (!isReadOnly)
That looks like the right approach to me.  If I had reviewing powers I'd r=me on
this, with the small change I mentioned.

Can someone review and check in?
Attached patch patch2 (obsolete) — Splinter Review
Updated patch, addressing comment 3.
Attachment #156743 - Attachment is obsolete: true
Comment on attachment 157009 [details] [diff] [review]
patch2

>+    if (type.EqualsLiteral("text") && (!isReadOnly) &&

I screwed up.  You just need !isReadOnly, without the brackets.  No need for a
new patch, though, that can be fixed on check-in.
Comment on attachment 157009 [details] [diff] [review]
patch2

Mike, is good to go?
Attachment #157009 - Flags: review?(mconnor)
Assignee: bugs → bryner
Flags: blocking-aviary1.0? → blocking-aviary1.0+
Not a "blocker" - if this gets review re-nominate. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
Comment on attachment 157009 [details] [diff] [review]
patch2

r=me, without the excess brackets as dean mentioned.  And since I'm here, lets
get this on the approval train.
Attachment #157009 - Flags: review?(mconnor)
Attachment #157009 - Flags: review+
Attachment #157009 - Flags: approval-aviary?
Re-nominating as per comment 8.
Flags: blocking-aviary1.0- → blocking-aviary1.0?
Comment on attachment 157009 [details] [diff] [review]
patch2

>--- nsFormFillController.cpp	20 Aug 2004 20:34:37 -0000	1.34
>+++ nsFormFillController.cpp	25 Aug 2004 23:43:42 -0000
>@@ -507,22 +507,25 @@ nsFormFillController::Focus(nsIDOMEvent*
> {
>   nsCOMPtr<nsIDOMEventTarget> target;
>   aEvent->GetTarget(getter_AddRefs(target));
>   
>   nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(target);
>   if (!input)
>     return NS_OK;
>     
>+    PRBool isReadOnly = PR_FALSE;
>+    input->HasAttribute(NS_LITERAL_STRING("readonly"), &isReadOnly);

This should be:

input->GetReadOnly(&isReadOnly);

or it won't pay attention to dynamic changes.
Attachment #157009 - Flags: superreview-
Attachment #157009 - Flags: approval-aviary?
I hand-edited Martijn's patch to make the two changes.	If it looks good, I'll
make sure it applies properly before checking in.
Attachment #157009 - Attachment is obsolete: true
Attachment #160678 - Flags: superreview?(bryner)
Attachment #160678 - Flags: review?(mconnor)
Comment on attachment 160678 [details] [diff] [review]
hand-edited patch

I'm going to carry forward Mike's r= on this.
Attachment #160678 - Flags: review?(mconnor) → review+
Summary: Clicking focused readonly textbox triggers autocompelte → Clicking focused readonly textbox triggers autocomplete
Attachment #160678 - Flags: superreview?(bryner) → superreview+
looks like patches are coming.  we should get this in quick.
Flags: blocking-aviary1.0? → blocking-aviary1.0+
Fixed on trunk and branch.
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
My VS.NET 2003 build just crashed and burned with

 /cygdrive/x/mozilla/toolkit/components/satchel/src/nsFormFillController.cpp
nsFormFillController.cpp
x:\mozilla\toolkit\components\satchel\src\nsFormFillController.cpp(525)
: error C2039: 'EqualsLiteral' : is not a member of 'nsAutoString'
        x:\obj\dist\include\string\nsTString.h(532) : see declaration of
 'nsAutoString'
make[6]: *** [nsFormFillController.obj] Error 2
make[6]: Leaving directory `/cygdrive/x/mozilla/obj/toolkit/components/satchel/s
rc'
make[5]: *** [libs] Error 2
Tinderbox builds have been building fine since this check-in, and this check-in
didn't add any new string comparisons.  Based on the line number, you're dying at:

  !autocomplete.LowerCaseEqualsLiteral("off"))

but that wasn't changed with this check-in.  I think it's something else that's
causing your problem.
Timo: see my previous comment
(In reply to comment #17)
> My VS.NET 2003 build just crashed and burned with
> 
>  /cygdrive/x/mozilla/toolkit/components/satchel/src/nsFormFillController.cpp
> nsFormFillController.cpp
> x:\mozilla\toolkit\components\satchel\src\nsFormFillController.cpp(525)
> : error C2039: 'EqualsLiteral' : is not a member of 'nsAutoString'
>         x:\obj\dist\include\string\nsTString.h(532) : see declaration of
>  'nsAutoString'
> make[6]: *** [nsFormFillController.obj] Error 2
> make[6]: Leaving directory `/cygdrive/x/mozilla/obj/toolkit/components/satchel/s
> rc'
> make[5]: *** [libs] Error 2

same error here, what can we do?
Erm, just compare this:
http://lxr.mozilla.org/aviarybranch/search?string=EqualsLiteral
(No matching files)

http://lxr.mozilla.org/seamonkey/search?string=EqualsLiteral
(Too many hits, displaying first 1000)

And no, there was no successful Firefox build since this checkin. Sweetlou and
Imalo are stuck.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Keywords: fixed-aviary1.0
Backed out on the branch.

You changed type.Equals to type.EqualsLiteral on the branch. But EqualsLiteral
isn't on the branch, see my comment above.

My build completes successfully with this backed out.
Well, it finally happened.  I broke a build.  My apologies to everyone for this
oversight.

I'm at work without my build machine.  Can someone check in this patch without
changing Equals to EqualsLiteral?  If not I'll do it tonight.
yeesh, I should have caught that, or bryner.  EqualsLiteral was introduced in
the 1.8a cycles by biesi.  Steffen, if you don't mind relanding this, I need to
get some code reviews done in a limited time window.. :)
I'll land this as soon as there's at least one green Firefox branch tinderbox.
(In reply to comment #0)
> 1. Load 
>   data:text/html,<form action="http://www.google.com/"><input name="q" readonly>
> 2. Click the textbox twice.
> 
> Result: Autocomplete widget appears.
> 
> You can even use autocomplete to put text into the field, which is invisible but
> submitted anyway.


Just stumbled upon this bug. I thought this was the desired behavior. I like it,
because you can find old search info there. I only wish there was a way to clear
it or "tune" it.
Comment on attachment 160755 [details] [diff] [review]
aviary patch, without EqualsLiteral

Bah.
Attachment #160755 - Attachment is obsolete: true
Attached patch aviary patchSplinter Review
This one compiles.
Comment on attachment 160760 [details] [diff] [review]
aviary patch

Based on comment 24, this can be checked in.
Checked into branch again 2004-10-01 14:43.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.0
Thanks for fixing my screw-up Steffen!
*** Bug 265079 has been marked as a duplicate of this bug. ***
*** Bug 265050 has been marked as a duplicate of this bug. ***
*** Bug 266278 has been marked as a duplicate of this bug. ***
*** Bug 263331 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.