Closed Bug 461855 Opened 13 years ago Closed 13 years ago

Places details pane retains height after opening tag selector or extended details

Categories

(Firefox :: Bookmarks & History, defect)

x86
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: Natch, Assigned: Natch)

Details

(Keywords: verified1.9.1)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3

Once you open either the tag selector or you hit more (on say a bookmark) the details pane remains at that height even after you click on say a separator which doesn't use the tag selector or the extended details (they aren't even available).

Reproducible: Always

Steps to Reproduce:
1.See details.
Possibly related bug 443155. This may be a problem with selectedIndex itself, which should automatically truncate the size.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is probably a Places bug...

Marco: should I do that hack from bug 416650 or is this really a more involved bug on selectedIndex?
Component: Bookmarks & History → Places
QA Contact: bookmarks → places
for now please attach the patch you already did
Attached patch A workaround (obsolete) — Splinter Review
Here's a modified patch to be consistent with the patch in bug 416650.
Attachment #347694 - Flags: review?(mano)
Comment on attachment 347694 [details] [diff] [review]
A workaround

Marco can you review this?
Attachment #347694 - Flags: review?(mano) → review?(mak77)
Assignee: nobody → highmind63
Status: NEW → ASSIGNED
Attachment #347694 - Flags: review?(mak77)
Comment on attachment 347694 [details] [diff] [review]
A workaround

>diff --git a/browser/components/places/content/places.js b/browser/components/places/content/places.js
>--- a/browser/components/places/content/places.js
>+++ b/browser/components/places/content/places.js
>@@ -642,6 +642,11 @@ var PlacesOrganizer = {
>   _fillDetailsPane: function PO__fillDetailsPane(aNodeList) {
>     var infoBox = document.getElementById("infoBox");
>     var detailsDeck = document.getElementById("detailsDeck");
>+    //XXX Natch hack to make sure the infoBox and tagsSelector UI don't add
>+    //    any height when we're not using them.

better comment please, not exactly an hack, we hide the infoBox if we don't need it, it is visible by default, but hidden on elements not supporting editing.

>+    var tagsSelector = gEditItemOverlay._element("tagsSelector");
>+    tagsSelector.hidden = false;
>+    infoBox.hidden = false; 
>     var aSelectedNode = aNodeList.length == 1 ? aNodeList[0] : null;
>     // If a textbox within a panel is focused, force-blur it so its contents
>     // are saved
>@@ -692,6 +697,8 @@ var PlacesOrganizer = {
>           itemsCountLabel.value =
>             PlacesUIUtils.getFormattedString("detailsPane.multipleItems",
>                                              [aNodeList.length]);
>+          infoBox.hidden = true;
>+          tagsSelector.hidden = true;

Is not the tagsSelector inside the infoBox? Hiding infoBox should hide it as well, isn't it?
Attached patch nits addressedSplinter Review
Comments addressed, you were right only the infobox was needed.
Attachment #347694 - Attachment is obsolete: true
Attachment #351083 - Flags: review?(mak77)
Comment on attachment 351083 [details] [diff] [review]
nits addressed

>diff --git a/browser/components/places/content/places.js b/browser/components/places/content/places.js
>--- a/browser/components/places/content/places.js
>+++ b/browser/components/places/content/places.js
>@@ -645,6 +645,10 @@ var PlacesOrganizer = {
>   _fillDetailsPane: function PO__fillDetailsPane(aNodeList) {
>     var infoBox = document.getElementById("infoBox");
>     var detailsDeck = document.getElementById("detailsDeck");
>+
>+    // Make sure the infoBox UI is visible if we need to use it, we hide it
>+    // below when we don't.
>+    infoBox.hidden = false; 

nit: trailing space
Attachment #351083 - Flags: review?(mak77) → review+
Comment on attachment 351745 [details] [diff] [review]
trailing space nuked.

this gets rid of a visual ui bug.
Attachment #351745 - Flags: approval1.9.1?
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/cbf0bbbcc8ed
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Version: unspecified → Trunk
Comment on attachment 351745 [details] [diff] [review]
trailing space nuked.

a191=beltzner
Attachment #351745 - Flags: approval1.9.1? → approval1.9.1+
verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090127 Shiretoko/3.1b3pre
Status: RESOLVED → VERIFIED
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.