595570 - Bookmark Sidebar no longer shows the Bookmark title when you hover over a link

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Ken]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre

With the latest build (Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100911041541):
The Bookmark Sidebar no longer shows the Bookmark title when you hover over a link.

Was working with previous builds.

Reproducible: Always

Steps to Reproduce:
1. Open Bookmark Sidebar
2. Move the Mouse pointer over a Link
3. Link title no longer displays.
Actual Results:  
Link title does not display.

Expected Results:  
Link title displays, like it still does when hovering over a Link in the Bookmarks Toolbar.

Comment 1

[Ken]

Attachment: Sample: Bookmark Sidebar no longer shows the Bookmark title

As seen in example, without a title it is impossible to tell which 'The Off... [icial Win32 ???? Trunk build is (or; is not yet) out]'.

Additional information about Sidebar use:
 I use the Sidebar most often since it remembers which Folders were open last time I used it and where I was in them.
 The other methods 'Show Bookmarks' even 'Bookmarks Toolbar' I have to drill down through my folders (often from 3 to 5 levels deep)...

 I use the Sidebar to select/maintain (Add/Delete) my Bookmarks.
 And if I am doing major reorganization of my Bookmark Folders/Links structure I will most often use the Sidebar along with 'Show all Bookmarks'.

Comment 2

[Alice0775 White]

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=29b9772d0c3a&tochange=c59be8574f7a

Comment 3

[Tyler Downer [He/Him]]

Seeing this as well. Possibly bug 588070?

Comment 4

[Alice0775 White]

I comfirme in local build that the changeset e6086627037c og buf 588070 causes the problem.

Comment 5

[[e-mail]]

Attachment: Set coordinates for tooltip event

Since we can't use a mouse event in this case, this was the easiest way I could find of propagating the coordinates to the popup showing event.

Comment 6

[Neil Deakin]

Comment on attachment 474520 [details] [diff] [review]
Set coordinates for tooltip event

Why is the first part of the patch there? Wasn't that the old version of the patch for bug 588070?

Comment 7

[[e-mail]]

(In reply to comment #6)
> (From update of attachment 474520 [details] [diff] [review])
> Why is the first part of the patch there? Wasn't that the old version of the
> patch for bug 588070?
Absolutely correct, that junk doesn't belong there at all. Before I attach a revised patch, do you agree with the approach?

Comment 8

[Neil Deakin]

Seems OK to me.

Comment 9

[[e-mail]]

Attachment: Without extraneous hunk

Comment 10

[Neil Deakin]

Comment on attachment 475234 [details] [diff] [review]
Without extraneous hunk

>diff --git a/layout/xul/base/src/nsXULPopupManager.cpp b/layout/xul/base/src/nsXULPopupManager.cpp
>--- a/layout/xul/base/src/nsXULPopupManager.cpp
>+++ b/layout/xul/base/src/nsXULPopupManager.cpp
>@@ -602,16 +602,27 @@ nsXULPopupManager::ShowTooltipAtScreen(n
>                                        PRInt32 aXPos, PRInt32 aYPos)
> {
>   nsMenuPopupFrame* popupFrame = GetPopupFrameForContent(aPopup, PR_TRUE);
>   if (!popupFrame || !MayShowPopup(popupFrame))
>     return;
> 
>   InitTriggerEvent(nsnull, nsnull, nsnull);
> 
>+  mCachedMousePoint = nsIntPoint(aXPos, aYPos);
>+  // coordinates are relative to the root widget
>+  nsPresContext* rootPresContext =
>+    popupFrame->PresContext()->GetRootPresContext();
>+  if (rootPresContext) {
>+    nsCOMPtr<nsIWidget> widget;
>+    rootPresContext->PresShell()->GetViewManager()->
>+      GetRootWidget(getter_AddRefs(widget));
>+    mCachedMousePoint -= widget->WidgetToScreenOffset();
>+  }
>+
>   popupFrame->InitializePopupAtScreen(aTriggerContent, aXPos, aYPos, PR_FALSE);
> 
>   FirePopupShowingEvent(aPopup, PR_FALSE, PR_FALSE);
> }
> 
> void
> nsXULPopupManager::ShowPopupWithAnchorAlign(nsIContent* aPopup,
>                                             nsIContent* aAnchorContent,

Comment 11

[[e-mail]]

Comment on attachment 475234 [details] [diff] [review]
Without extraneous hunk

Yes, it's a regression fix for a regression fix. Sorry about that.

Comment 12

[Nochum Sossonko [:Natch]]

Comment on attachment 475234 [details] [diff] [review]
Without extraneous hunk

It's a blocker!

Comment 15

[[e-mail]]

Pushed changeset b5744e9dd0f0 to mozilla-central.