395174 - Identity pop-up should disappear as soon as I start typing in the location bar

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[Ria Klaassen (not reading all bugmail)]

Steps to reproduce:

- Click on the favicon in the URLbar in order to select the field, the identity popup will appear
- Start typing, several history autocomplete pop-ups will appear and overlap the indentity popup
- When the autocompletes stop appearing you'll still see the identity pop-up of the site you planned to leave and that's illogical

Maybe one would say: don't click on the favicon if you want to leave, but it is the only way that ALWAYS selects the location bar field.

Comment 1

[Ria Klaassen (not reading all bugmail)]

The pop-up also stays after hitting enter and while the new site is loading (although the information in the pop-up changes) and it only disappears when you click in the site.

Comment 2

[Reed Loden [:reed]]

See bug 395314.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

This is similar to, but not identical to 395314; IMO, as soon as the URL field has changed, Larry should no longer report website identity, since it no longer relates to the address that the user is viewing.

Comment 4

[Mike Connor [:mconnor]]

agreed.  would like to fix for M9

Comment 5

[Dão Gottwald [:dao]]

see also bug 400935

Comment 6

[Johnathan Nightingale [:johnath]]

Attachment: hide identity box and popup on input

So this patch does both things mentioned in the bug:
 a) hide the popup, if it's showing, when the user types in the location bar (this was the bug as originally reported)
 b) hide the identity box as well, when the user types.

Doing a) makes perfect sense, but having it here and interacting with it, b) feels weird.  It causes the location bar to jump around while you're typing - and even though the caret does the right thing, the shifting text is still a little jarring.

I think part of what makes it weird is that we don't have any precedent - we don't, for instance, take away the yellow background on input.  Maybe that should happen too, but the current behaviour feels "off" somehow.  I'm open to suggestions here, since beltzner and mconnor both seemed to support this behaviour - maybe I'm alone in thinking this way?

As you can see from the patch, it would be trivial to modify it to do only a), if we felt that was appropriate/preferable.

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #6)
> we don't, for instance, take away the yellow background on input.

Bug 398020 does that.
But IMHO doing that isn't really related to the problem that occurs if you remove the box as the user starts to type.

Comment 8

[Ian Pottinger]

Is it possible to blank the contents of the identity box when edit begins, then then wait until the user hits enter or clicks the go button or selects an auto-complete entry or whatever other actions are associated with end-of-edit.  At that point if the url is effectively unchanged then restore the contents else hide the box and let the new page load.

Comment 9

[Ian Pottinger]

Drat! Forgot to mention that by "blank" I mean hide the context of the identity box while leaving its dimensions the same. (text colour set to background colour?)

Comment 10

[Johnathan Nightingale [:johnath]]

Attachment: Just hide the popup

This patch just hides the popup, but leaves the identity box untouched, per the original problem report.  Bug 401560 filed to track the broader discussion about whether removing SSL indicators on locbar-input is desireable.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 286557 [details] [diff] [review]
Just hide the popup

Could consolidate input event handlers by adding the code from the URL bar's oninput to this new handler.

Comment 12

[Johnathan Nightingale [:johnath]]

Attachment: Pull oninput code into the new handler

Per Gavin's suggestion, this patch removes the oninput from browser.xul and brings the one-liner into the URLBarOnInput handler.

Comment 13

[Dão Gottwald [:dao]]

Doesn't the listener need to be added in BrowserToolboxCustomizeDone, too?

Comment 14

[Johnathan Nightingale [:johnath]]

Attachment: Register handler using xul attribute instead of addEventListener call

As Dao mentioned, registering the input handler in DelayedStartup means also having to register it in other places, like BrowserToolboxCustomizeDone.  This is actually a bug for the drag and drop handlers too, filed bug 401576.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 286568 [details] [diff] [review]
Register handler using xul attribute instead of addEventListener call

>+                   oninput="return URLBarOnInput(event);"

IMHO the |return| should be removed. URLBarOnInput doesn't return anything, and if needed there's the preventDefault event method.

Comment 16

[Johnathan Nightingale [:johnath]]

Attachment: Don't return in markup when there's no return value from the handler

Dao's right, I think - don't make it look like oninput is going to return when the handler doesn't.

Comment 17

[Reed Loden [:reed]]

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.878; previous revision: 1.877
done
Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.385; previous revision: 1.384
done

Comment 18

[Ria Klaassen (not reading all bugmail)]

Verified. Great!

Comment 19

[Dão Gottwald [:dao]]

Stephen, what's the litmus test for this?

Is the patch that has been checked in still making a difference? That is, can you actually type when the popup is open? I can't.

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #19)
> Stephen, what's the litmus test for this?

Sorry, I should've posted it: https://litmus.mozilla.org/show_test.cgi?id=5301.  I originally tried to add this case to https://litmus.mozilla.org/show_test.cgi?id=4728, but they're distinct enough that I created the new one.

> Is the patch that has been checked in still making a difference? That is, can
> you actually type when the popup is open? I can't.

It doesn't work for me, either, using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre.

I think this has changed from what Ria saw in comment 0 because we no longer select the URL bar on favicon/Larry click, right?  Should we reopen, or file a new bug? 

Comment 21

[Dão Gottwald [:dao]]

(In reply to comment #20)
> I think this has changed from what Ria saw in comment 0 because we no longer
> select the URL bar on favicon/Larry click, right?

Yes, and that's intentional: bug 400935. So no new bug is needed.
We should probably back out this bug's patch, since it's just unneeded overhead, and resolve this bug as invalid -- it was valid when it was filed, but the steps to reproduce don't work anymore.

Comment 22

[Dão Gottwald [:dao]]

Attachment: revert patch

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318238 [details] [diff] [review]
revert patch

Can we get confirmation that this isn't needed on Linux? IIRC popup behavior there is special and might require this kind of hack.

Comment 24

[Dão Gottwald [:dao]]

Tested on Linux: opening the identity popup doesn't focus the address field, so you can't type there.

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318238 [details] [diff] [review]
revert patch

>Index: browser/base/content/browser.xul

>+                 oninput="gBrowser.userTypedValue = this.value"

Nit: add a trailing ";".

r=me, but I don't think we should land this until we branch/switch to Hg/whatever.

Comment 26

[Dão Gottwald [:dao]]

Attachment: revert patch + nit fix

Comment 27

[Dão Gottwald [:dao]]

revert patch landed:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/5fd36522851e