Closed
Bug 364075
Opened 18 years ago
Closed 18 years ago
Separator sometimes is sometimes displayed as generic bookmark in bookmarks bar
Categories
(Camino Graveyard :: Bookmarks, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: froodian, Assigned: froodian)
Details
(Keywords: fixed1.8.1.1)
Attachments
(1 file, 2 obsolete files)
|
1.02 KB,
patch
|
stuart.morgan+bugzilla
:
superreview+
|
Details | Diff | Splinter Review |
STR: 1. Cmd-B 2. Select Bookmarks bar collection, make sure no bookmark items are selected 3. + menu, Add Separator 4. Click on a bookmark item other than the separator 5. Click on the separator What happens: Separator is now displayed as a generic bookmark favicon in the bm bar. What should happen: Should still look like a separator
|
Assignee | |
Comment 1•18 years ago
|
||
The problem here was that we were setting the icon and title in |bookmarkChanged:|. This solution could be considered a little excessive (a simple "if ([mItem isSeparator]) return NO;" at the beginning of the method would suffice), but I like it because it encapsulates the separator display logic.
|
|
Assignee | |
Comment 2•18 years ago
|
||
Oops, forgot the declare.
Attachment #248857 -
Attachment is obsolete: true
Attachment #248858 -
Flags: review?(bugzilla)
Attachment #248857 -
Flags: review?(bugzilla)
|
||
Comment 3•18 years ago
|
||
Comment on attachment 248858 [details] [diff] [review] Patch Yeah, that looks good to me.
Attachment #248858 -
Flags: superreview?(stuart.morgan)
Attachment #248858 -
Flags: review?(bugzilla)
Attachment #248858 -
Flags: review+
|
||
Comment 4•18 years ago
|
||
Comment on attachment 248858 [details] [diff] [review] Patch I don't like displayAsSeparator being a hybrid of something you call to make something look like a separator and something that tells you if a bookmark is a separator. The solution you mention in comment 1 sounds cleaner.
Attachment #248858 -
Flags: superreview?(stuart.morgan) → superreview-
|
|
Assignee | |
Comment 5•18 years ago
|
||
A slightly more reasonable solution. ;)
Attachment #248858 -
Attachment is obsolete: true
Attachment #248870 -
Flags: superreview?(stuart.morgan)
|
|
||
Comment 6•18 years ago
|
||
Comment on attachment 248870 [details] [diff] [review] Patch sr=smorgan
Attachment #248870 -
Flags: superreview?(stuart.morgan) → superreview+
|
|
Assignee | |
Comment 7•18 years ago
|
||
Checked in on 1.8branch and trunk
You need to log in
before you can comment on or make changes to this bug.
Description
•