430213 - Fill details pane when folders in the left tree of the Library are selected

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[MZ (watervole02)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042115 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042115 Minefield/3.0pre

The properties editor is shown when a page is selected in the library's display area, but it isn't shown when folders in the left tree pane are selected. I think selecting folders in the pane should bring up the properties editor because it would make the process of renaming folders more intuitive compared to the current way (going to the parent folder and clicking on a sub folder). It would also decrease the number of clicks required to edit an item's properties.

Reproducible: Always

Steps to Reproduce:
1. Go to the "Bookmarks" menu
2. Click "Organize Bookmarks..."
3. Click a folder in the left tree pane
Actual Results:  
Only the number of items in the folder and some directions are shown in the display area.

Expected Results:  
The properties editor should be shown in the display area along with the number of items and the directions.

Comment 1

[MZ (watervole02)]

Attachment: Current view when folder is selected

Comment 2

[MZ (watervole02)]

Attachment: Proposed mockup

Comment 3

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042209 Minefield/3.0pre

Confirmed that no properties are shown in the bottom panel. I don't know if this is intended or just forgotten. 

Comment 4

[Marco Bonardo [:mak]]

imo the details panel is right pane "devoted", but since now we don't have properties context in the left pane, this could alleviate the problem

Comment 5

[MZ (watervole02)]

(In reply to comment #4)
> imo the details panel is right pane "devoted", but since now we don't have
> properties context in the left pane, this could alleviate the problem
> 

What about placing the properties editor in a display area at the bottom? That way it won't be specific to the right or left panes. The editor then looks kind of large though, even if some text is moved around.

Comment 6

[MZ (watervole02)]

Attachment: Properties editor in separate area

Comment 7

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

(In reply to comment #4)
> imo the details panel is right pane "devoted", but since now we don't have
> properties context in the left pane, this could alleviate the problem

I don't think that's really clear to the user, and it's confusing that selection in the left view will modify the top of the right view, but not the bottom of the right view.

We should fix this. Not a blocker, but wanted.

(Putting the properties across the entire bottom pane is probably a good idea, but not as part of this bug, and requires rethinking of the layout of the controls in that area.)

Comment 8

[Marco Bonardo [:mak]]

(In reply to comment #7)
> (Putting the properties across the entire bottom pane is probably a good idea,
> but not as part of this bug, and requires rethinking of the layout of the
> controls in that area.)

that would appear more correct, but will leave a lot of unused space in the properties panel (until we don't have the site preview, that could fill space well).
We should also consider that the screen there is taken with a few folders in the left pane but that could grow quite large, having less space in a tree could potentially make harder recognize parents / children of folders

I agree that this is surely wanted, but imo blocker if we don't reinsert properties context menu in the left pane items.

Comment 9

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

Yeah, I agree, either this or bug 430075 needs to block, and this is more discoverable.

Comment 10

[Marco Bonardo [:mak]]

Attachment: patch

we should update the details pane in these cases:
- focus changes between the trees
- user clicks on an item
- selection changes

but we should also try avoiding update the details pane if we are already editing the same item.

Comment 11

[Marco Bonardo [:mak]]

mh, i did a mistake in the comment, we should update the details pane in these cases:
- focus changes between the trees
- selection changes in the left tree
- selection changes in the right tree

Comment 12

[Asaf Romano (gone)]

It looks like you no longer null check selectedNode... (which can happen, esp. in the right pane).

And please make it aSelectedNode.

Comment 13

[Marco Bonardo [:mak]]

there are the same checks as before on selectedNode

all three points are calling fillDetailsPane, that does a 
+      if (selectedNode && gEditItemOverlay.itemId == selectedNode.itemId &&

then after you have again      
     if (selectedNode && !PlacesUtils.nodeIsSeparator(selectedNode)) {

i have to call fillDetailsPane also if selectedNode is null

Comment 14

[Asaf Romano (gone)]

Comment on attachment 317553 [details] [diff] [review]
patch

yeah, i read that wrong, r=mano (please do s/selectedNode/aSelectedNode/).

Comment 15

[Marco Bonardo [:mak]]

yep, will come with fixed local var, thank you

Comment 16

[Marco Bonardo [:mak]]

Attachment: patch

fixed local var name

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 317553 [details] [diff] [review]
patch

a=mconnor on behalf of 1.9 drivers

Comment 18

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

mozilla/browser/components/places/content/places.js 	1.161
mozilla/browser/components/places/content/places.xul 	1.133 

Comment 19

[Tracy Walker [:tracy]]

verified with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre

Comment 20

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

This has caused two regressions: bug 431528 and bug 430213

Comment 21

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

Er, and bug 431140, rather.

Comment 22

[Gervase Markham [:gerv]]

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