Location Bar Overflow-Tooltip is off by 3 addresses

RESOLVED FIXED

Status

SeaMonkey
Location Bar
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: cooperg, Assigned: Son Le)

Tracking

(Depends on: 1 bug)

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

2.21 KB, patch
neil@parkwaycc.co.uk
: review+
chris hofmann
: approval1.8b2+
Details | Diff | Splinter Review
2.70 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624

When mousing over a very long URL in the Location Bar's history-dropdown the 
URL it displays is off by 3 addresses. 

Reproducible: Always

Steps to Reproduce:
1. In the URL bar type "google". 

(Assuming you use Google a lot,) Mozilla will automatically bring up a snazzy
dropdown of all your old searches. Unfortunately not much shows up regarding
what you actually searched for (~20 chars) so you have to mouse over the URL to
see what the rest of your search was. 

2. Mouse over the URLs in the list. 

Notice that the first 3 URLs have no tooltip entry. Starting with the 4th URL
the tooltips appear, but the indexes are off, so the 4th entry gets the 1st
entry's tooltip!
(Assignee)

Comment 1

13 years ago
Confirmed
(Assignee)

Comment 2

13 years ago
Created attachment 181967 [details] [diff] [review]
patch v0

fix and a bit of a cleanup (remove unused view)
Attachment #181967 - Flags: review?(neil.parkwaycc.co.uk)

Comment 3

13 years ago
Comment on attachment 181967 [details] [diff] [review]
patch v0

bz, this is basically the same issue as the autocomplete mouseover bug, with
the difference that this code is also used for non-popup trees. Do you want
this hack or do you want to fix popup coordinates?

>+  if (!mSourceNode)
>+    return;
>+
>+  nsCOMPtr<nsIBoxObject> bx;
>+  nsCOMPtr<nsIDOMXULElement> xulEl(do_QueryInterface(mSourceNode));
>+  xulEl->GetBoxObject(getter_AddRefs(bx));
You don't actually have to jump though these hoops, because
GetSourceTreeBoxObject already did the legwork for you.

>+
>   nsCOMPtr<nsITreeBoxObject> obx;
>   GetSourceTreeBoxObject(getter_AddRefs(obx));
>-  if (obx) {
      nsCOMPtr<nsIBoxObject> bx(do_QueryInterface(obx)); should suffice.
Attachment #181967 - Flags: superreview?(bzbarsky)
Attachment #181967 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #181967 - Flags: review+
I think in the short term we need the hack, since fixing popup coordinates is an
alpha-type change...

That said, the patch looks wrong to me, since the x and y on boxObject aren't
exactly client coords.  Shouldn't the right thing be to just take the event
screen coords and subtract the screen coords of the boxObject for the
documentElement of the document the target tree is in?

Especially so because it's not clear to me whether bx and obx are even required
to be in the same document.
Comment on attachment 181967 [details] [diff] [review]
patch v0

r-.  To see why, just put borders on some things (say a 100px border on the
root element of the document the tree is in) and watch the x/y boxObject coords
screw you over.
Attachment #181967 - Flags: superreview?(bzbarsky) → superreview-

Comment 6

13 years ago
Ah, so the fix for revision 1.114 of autocomplete.xml actually works generically!
Yes, it does.  Part of the reason I went with it... ;)
(Assignee)

Comment 8

13 years ago
Created attachment 182313 [details] [diff] [review]
patch v1

changed to subtracting the screen coords of the boxObject for the
documentElement
Attachment #181967 - Attachment is obsolete: true
Attachment #182313 - Flags: superreview?(bzbarsky)
Attachment #182313 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 182313 [details] [diff] [review]
patch v1

Let me know if you need this checked in, ok?
Attachment #182313 - Flags: superreview?(bzbarsky) → superreview+

Comment 10

13 years ago
Comment on attachment 182313 [details] [diff] [review]
patch v1

This works, except there appears to be an include missing:
@@ 50,6 +50,7 @@
 #include "nsIServiceManager.h"
 #ifdef MOZ_XUL
 #include "nsITreeView.h"
+#include "nsIDOMNSDocument.h"
 #endif
 #include "nsGUIEvent.h"
 #include "nsIPrivateDOMEvent.h"
Attachment #182313 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Comment 11

13 years ago
Created attachment 182398 [details] [diff] [review]
patch v1.1

Added missing include (it was in the header file which I forgot to include).

bzbarsky, if you could check it in that would be great!
(Assignee)

Comment 12

13 years ago
Comment on attachment 182313 [details] [diff] [review]
patch v1

make XUL tooltips work
Attachment #182313 - Flags: approval1.8b2?

Comment 13

13 years ago
Comment on attachment 182313 [details] [diff] [review]
patch v1

a=chofmann
Attachment #182313 - Flags: approval1.8b2? → approval1.8b2+
Assignee: hewitt → son.le0
Status: UNCONFIRMED → NEW
Ever confirmed: true
Patch checked in for 1.8b2.  Thanks for fixing this!
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
*** Bug 269522 has been marked as a duplicate of this bug. ***
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.