346772 - First item in History search results has all disabled items in context menu

Status: RESOLVED FIXED

(Camino Graveyard :: History, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

STR:

1. Open History
2. Search for something you know will have at least 1 result
3. Select the first (top-most) result and try to ctrl-click or Action-menu it

Expected: Active CM items
Actual: All CM items disabled

This also happens if you select the first item and any additional item.

What I think is happening is that somehow the cm-control is still thinking that the "Today" folder is the first item in the search results (we don't allow CMs on folders in history) but also knows it's a history item and getting very confused...  Observe the grey-highlight in the search results if you have 1) the Today folder selected before searching 2) if you have the first/second history *item* selected before searching....

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** Bug 362642 has been marked as a duplicate of this bug. ***

Comment 2

[froodian (Ian Leue)]

Attachment: Patch

|selectedRowEnumerator| is deprecated.  This uses the preferred |selectedRowIndexes| instead (which fixes the bug).

Comment 3

[Stuart Morgan]

Comment on attachment 247342 [details] [diff] [review]
Patch

Ew; thanks Apple.  What an ugly recommended enumeration scheme. Anyway, the bug doesn't actually have anything to do with selectedRowEnumerator being deprecated, it's because 
  while ((currentRow = [[rowEnum nextObject] intValue]))
stops if the first selected row has an index of 0. I think I fixed this bug for Bookmarks a while back, and apparently didn't realize history had parallel code.

This version does a lot more work than it needs to in most cases. Given that we are usually going to be able to stop very quickly, I'd rather see a loop based on firstIndex and indexGreaterThanIndex:, keeping the early return.

Comment 4

[froodian (Ian Leue)]

Attachment: Patch

Yeah, it's an ugly scheme.  firstIndex and indexGreaterThanIndex is better though, thanks.

Comment 5

[Stuart Morgan]

Comment on attachment 247359 [details] [diff] [review]
Patch

r=me

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 247359 [details] [diff] [review]
Patch

sr=pink

Comment 7

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.