431892 - Tag selection UI does not collapse in Library

Status: RESOLVED DUPLICATE of bug 416650

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak]]

reported on mozillazine forum:

1. Open the Library.
2. Click on a bookmark with tags and expand the tag box. Now click on any item in the treeview on the left.
3. The expanded tag box won't go away.

2. is valid also if you click on a folder in the right pane

confirmed, needs to block

Comment 1

[Tracy Walker [:tracy]]

> confirmed, needs to block

The only way to get rid of the expanded tag box is to close then reopen the Library

Comment 2

[Bob Uliss]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050209 Minefield/3.0pre

Confirmed - on MAC os x 10.5.2 G5

Comment 3

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

This is WFM on Windows, fwiw. I don't think it blocks, but it does suck!

Related to bug 431817 or bug 431817 comment 10 ?

Comment 4

[Tracy Walker [:tracy]]

broken for me with latest nightly builds of:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050204 Minefield/3.0pre
and 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050206 Minefield/3.0pre

Comment 5

[Matthew Babbs]

This also occurs when selecting a folder in the right pane.

Comment 6

[Marco Bonardo [:mak]]

(In reply to comment #3)
> This is WFM on Windows, fwiw. I don't think it blocks, but it does suck!

for me it is confirmed on Windows...

Comment 7

[Marco Bonardo [:mak]]

Attachment: patch

Hide the tag selector if the item does not accept tag.

This also fixes the location field for history items, it should be showed, it's quite important showing the url of an history item (until it is not a query), regardless it being bookmarked or not.

Comment 8

[Asaf Romano (gone)]

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

you should only show the tags selector if it was already shown (i.e. it's collapsed by default).

Comment 9

[Marco Bonardo [:mak]]

indeed i'm changing .hidden attribute to avoid touching up the .collapsed status, so if hidden is true collapsed can still be false. this simply hide when we can't take a tag, regardless the collapsed status of the pane. Do you think that this could be confusing?

Comment 10

[Asaf Romano (gone)]

Yeah, I think it does, I would rather make the code self-describing...

Comment 11

[Marco Bonardo [:mak]]

i still have doubt about what you're thinking about, would this be more clear(?):
    this._element("tagsSelector").hidden =
      !this._element("tagsSelector").collapsed &&
      this._element("tagsRow").collapsed;

Comment 12

[Asaf Romano (gone)]

something like
 if (!this._element("tagsSelector").collapsed)
   this._element("tagsSelector").collapsed =  this._element("tagsRow").collapsed;

Comment 13

[Marco Bonardo [:mak]]

so you change the collapsed status of the pane, so if you open tha tag selector UI, go to a folder, then come back to a bookmark you have to reopen the tags pane, is that wanted or a typo?

Comment 14

[Marco Bonardo [:mak]]

Attachment: patch

while waiting for an answer, this should increase code readability

Comment 15

[Asaf Romano (gone)]

(In reply to comment #13)
> so you change the collapsed status of the pane, so if you open tha tag selector
> UI, go to a folder, then come back to a bookmark you have to reopen the tags
> pane, is that wanted or a typo?
> 

We don't persist it elsewhere (the bookmarking panel), so I think it's wanted.

Comment 16

[Marco Bonardo [:mak]]

Attachment: patch

As you will, notice however that the selector retain its status while moving trough bookmarks, while with this patch if you scroll down with keyboard in a folder the status is retained until you move over bookmarks, then after the first folder it will be collapsed for all next items and imo that appear quite strange.

Also we have to use ToggleTagsSelector() because the expand button has to be updated correctly.

Comment 17

[Marco Bonardo [:mak]]

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

most likely needs an unbitrot

Comment 19

[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