130448 - urlbar popup mislocated; if url.length > ~170 chars is persisted to 'nc:urlbar-history'

Status: VERIFIED FIXED

(SeaMonkey :: Location Bar, defect)

Description

[John Morrison]

If you have a URL showing in the urlbar, and you press Enter in the urlbar, then 
this urlbar will appear as one of the selections in the urlbar-history popup. 
(Okay, that part's normal).

The bug is: if that URL is fairly long (somewhere more than 170 characters; think 
bugzilla URLs, shopping URLs, search URLs), then, after that is persisted to
urlbar-history, the popup for the history dropdown of the urlbar will be located
to the left of the location of the urlbar. (The longer it is, the more to the
left it is misplaced, until final it appears flush with the left hand edge of
the screen.)

Steps to reproduce:
1) shutdown browser; rename localstore.rdf to foo.rdf; startup again (creates a
clean localstore.rdf)
2) put the cursor in the urlbar and hit Enter; this puts the url of your home
page in the urlbar-history
3) hit history dropdown; note location is ok.
4) click on the URL for this bug (about 202 characters long); put focus in
urlbar and hit Enter. Hit history dropdown. Note that it is now located off to
the left of its proper location.

Comment 1

[John Morrison]

nsbeta1, with a tepid recommendation (i.e., it isn't the worst problem in the
world, but damn it looks broken).

Comment 2

[John Morrison]

... but I will note that I "discovered" this in normal use of the browser. I had 
a newspaper URL that was about this long 'stuck' in my urlbar-history for the past 
two days with this bug, and the broken popup along for the ride. This isn't the 
by-product of going off looking for odd bugs :-).

Comment 3

[Warner Young]

When this occurs, where the URL dropdown appears seems to be about the same no
matter where your browser window is positioned.  So how far "off" it appears
would depend on how close to the left edge of the screen your browser is.

Comment 4

[Peter Trudelle]

I see this a lot. very visible regression. nsbeta1+

Comment 5

[Peter Trudelle]

adt1/1.0, very visible regression.

Comment 6

[Joe Hewitt (gone)]

*** Bug 132074 has been marked as a duplicate of this bug. ***

Comment 7

[Peter Trudelle]

I haven't seen this in recent builds.  Is anyone else still seeing it?

Comment 8

[John Morrison]

Yes, I can still reproduce using the url for this bug, and the steps outlined 
above. (current trunk build win2k)

Comment 9

[Peter Trudelle]

I can still repro too, but I'm not seeing it in normal use.  Is anyone else?

Comment 10

[John Morrison]

Okay, in _my_ normal use, I will bring up the "New Checkins" link on the 
mozilla.org home page. At some point, I may set focus in the urlbar and
hit Enter to get a newer version of the checkin list. That will trigger 
this bug. (One may (or probably will) argue that I'm not normal; I plead
/nolo contendere/).

Comment 11

[Peter Trudelle]

lowering impact to ADT2, we may have to suffer with this for beta.

Comment 12

[Thomas Schoepf]

This bug is not limited to Windows 2000. I'm seeing it both on Windows Me and
Linux. I need to change long urls by hand every now and then, so I see this bug
quite often.

One thing I noticed about the offset: This offset seems to be related to the
length of the URL: It seems to be half the difference of URL.length.pixels and
Location.window.width.pixels.

e.g. URL is 200 pixels, location window is 100 pixels then the location window
will be offset (200 - 100) / 2 = 50 pixels to the left of the screen.

Comment 13

[Joe Hewitt (gone)]

What's happening here is that the popup is being positioned so that the entire
width of the widest menuitem can fit on screen, as a popup normally would be. 
However, since we crop the popup with the fit the urlbar width, it shouldn't
need to do this.  Calculations must be happening in the wrong order.

Comment 14

[Joe Hewitt (gone)]

Attachment: patch

In the case where sizetopopup is specified, I just insist that the popup be
aligned with the left coordinate of its parent in addition to having the same
width.

Comment 15

[Thomas Schoepf]

Attachment: patch

Hello Joe, your patch did not work for me. The urlbar window still had an
offset, now to the right of the screen and sometimes showed up in the upper
left corner of the screen.

The attached patch (against mozilla-1.0rc1) works for me. My logic is: instead
of moving the window away and then moving it back again, prevent the moving in
the first place.

HTH

Comment 16

[Thomas Schoepf]

There's another issue: Move the browser to the left so that the urlbar is partly
off the screen. Then open the popup and it will be positioned at screen coords
(0,0). The same happens with the main menu (File, Edit, View ...)

Comment 17

[Thomas Schoepf]

Attachment: patch (obsoletes my previous patch)

This patch keeps the width if sizedtopopup is set.
Additionally it fixes the misplacement bug when the parent was moved off the
screen to the left or to the top.
This also means that the urlbar and toolbar menus always will be fully visible.
IMO it does not make sense to stick to the parent if it's (partly) off the
screen.

Comment 18

[Joe Hewitt (gone)]

*** Bug 143100 has been marked as a duplicate of this bug. ***

Comment 19

[Michael Sinz]

Yup, this sounds like my bug.  I guess I did not find this one in my search. 
Sorry for the dup.

Comment 20

[Joe Hewitt (gone)]

Comment on attachment 82677 [details] [diff] [review]
patch (obsoletes my previous patch)

This patch is much better, but can you move this code:

+    if (tag.get() != nsXULAtoms::tooltip &&
+      nsMenuFrame::IsSizedToPopup(parentContent, PR_FALSE))


into a boolean variable so we don't have to check it twice?

Comment 21

[Thomas Schoepf]

Attachment: Compute sizedToPopup only once (obsoletes previous two patches)

Comment 22

[R.K.Aa.]

Related: bug 125081, bug 140557

Comment 23

[Blake Ross]

*** Bug 132245 has been marked as a duplicate of this bug. ***

Comment 24

[Thomas Schoepf]

I'd like to request a review of attachment 83036 [details] [diff] [review]
Thanks

Comment 25

[Joe Hewitt (gone)]

*** Bug 126124 has been marked as a duplicate of this bug. ***

Comment 26

[Joe Hewitt (gone)]

Comment on attachment 83036 [details] [diff] [review]
Compute sizedToPopup only once (obsoletes previous two patches)

sr=hewitt

Comment 27

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 83036 [details] [diff] [review]
Compute sizedToPopup only once (obsoletes previous two patches)

r=[e-mail]

Comment 28

[Joe Hewitt (gone)]

fixed on trunk

Comment 29

[John Morrison]

verified on trunk.

Comment 30

[John Morrison]

*** Bug 125081 has been marked as a duplicate of this bug. ***

Comment 31

[Peter Trudelle]

nominating for branch landing.

Comment 32

[Joris Huizer]

Using the Mozilla 1.0 Release Candidate 3, I still see this behavior - when is
the fix implemented ?

Comment 33

[R.K.Aa.]

*** Bug 147112 has been marked as a duplicate of this bug. ***

Comment 34

[Thomas Schoepf]

Can someone please nominate this for mozilla1.0.1 branch landing?
Thanks

Comment 35

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) for checkin to the 1.0 branch, pending Driver's
approval.

Comment 36

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 83036 [details] [diff] [review]
Compute sizedToPopup only once (obsoletes previous two patches)

please check into the 1.0.1 branch ASAP. once landed remove the 
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword

Comment 37

[Thomas Schoepf]

Can someone please check this into the branch? Thank you!

Comment 38

[R.K.Aa.]

*** Bug 148603 has been marked as a duplicate of this bug. ***

Comment 39

[Mike Kowalski]

fixed on branch.

Comment 40

[Matthias Versen [:Matti]]

*** Bug 151702 has been marked as a duplicate of this bug. ***

Comment 41

[Claudius Gayle]

VERIFIED Fixed 20020711 branch builds

Comment 42

[Vidar Haarr (not reading bugmail)]

*** Bug 157920 has been marked as a duplicate of this bug. ***

Comment 43

[R.K.Aa.]

*** Bug 162408 has been marked as a duplicate of this bug. ***