Last Comment Bug 586415 - Some css rules in existing css files doesn't apply anymore (bookmarks-ptf gone, structure have changed)
: Some css rules in existing css files doesn't apply anymore (bookmarks-ptf gon...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: seamonkey2.1b1
Assigned To: Stefan [:stefanh]
:
Mentors:
Depends on:
Blocks: SMPlacesBMarks
  Show dependency treegraph
 
Reported: 2010-08-11 13:30 PDT by Stefan [:stefanh]
Modified: 2010-08-27 15:45 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix navigator.css in all themes (8.89 KB, patch)
2010-08-12 14:50 PDT, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Now including bookmarksToolbar.css (9.73 KB, patch)
2010-08-15 13:17 PDT, Stefan [:stefanh]
neil: review-
Details | Diff | Splinter Review
Now without removed rules (4.79 KB, patch)
2010-08-26 13:24 PDT, Stefan [:stefanh]
neil: review+
neil: superreview+
Details | Diff | Splinter Review

Comment 1 Stefan [:stefanh] 2010-08-11 14:10:00 PDT
Fixing this will also fix bug 586026, comment #10
Comment 2 Stefan [:stefanh] 2010-08-11 14:17:29 PDT
Note also that rules (navigator.css) like "#personal-bookmarks > .bookmarks-toolbar-customize" doesn't work anymore.
Comment 3 Robert Kaiser 2010-08-11 18:02:52 PDT
Some of this was noted in the comments I pointed to in bug 585601 and I had in mind to fix it there, but unfortunately, landing places bookmarks hasn't decreased my TODO list, but made it longer and since then I couldn't come to do much real work, for whatever reasons. I hope to get to this some time soon, but might not be this week.
Comment 4 Stefan [:stefanh] 2010-08-12 14:50:20 PDT
Created attachment 465391 [details] [diff] [review]
Fix navigator.css in all themes

I think this is all that needs to be done (assuming this just affected navigator.css).

While fixing this I noted some nits that needs to be fixed in the mac version, bit I'll file a follow-up on that.
Comment 5 neil@parkwaycc.co.uk 2010-08-13 16:48:29 PDT
Comment on attachment 465391 [details] [diff] [review]
Fix navigator.css in all themes

> /* Prevent [mode="icons"|"text"] from hiding the label and icon */
>-#bookmarks-ptf .bookmark-item > .toolbarbutton-text,
>-#bookmarks-ptf .bookmark-item > .toolbarbutton-icon {
>+#PlacesToolbar .bookmark-item > .toolbarbutton-text,
>+#PlacesToolbar .bookmark-item > .toolbarbutton-icon {
>   display: -moz-box !important;
KaiRo added an inferior rule to BookmarksToolbar.css :-(
Comment 6 Robert Kaiser 2010-08-14 15:59:02 PDT
(In reply to comment #5)
> KaiRo added an inferior rule to BookmarksToolbar.css :-(

Why inferior? Is there any case where .bookmark-item is not inside a #PlacesToolbar and needs to follow those modes?
Comment 7 Stefan [:stefanh] 2010-08-15 13:17:59 PDT
Created attachment 466161 [details] [diff] [review]
Now including bookmarksToolbar.css
Comment 8 neil@parkwaycc.co.uk 2010-08-15 14:11:20 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > KaiRo added an inferior rule to BookmarksToolbar.css :-(
> Why inferior?
You went to all the trouble of making favicons work in bookmarks, and then you let people turn them off ;-)
Comment 9 Robert Kaiser 2010-08-15 14:25:33 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > KaiRo added an inferior rule to BookmarksToolbar.css :-(
> > Why inferior?
> You went to all the trouble of making favicons work in bookmarks, and then you
> let people turn them off ;-)

Well, for one thing, I heard people talk of not being able to turn them off as a bug, but OK, I see that side - the rule itself without the space selector is superior than the unneededly longer rule in bookmarks.css though :P
Comment 10 neil@parkwaycc.co.uk 2010-08-22 03:41:47 PDT
Comment on attachment 466161 [details] [diff] [review]
Now including bookmarksToolbar.css

>-toolbarpaletteitem[place] > #personal-bookmarks > .bookmarks-toolbar-customize {
>+toolbarpaletteitem[place] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-customize {
Eww, this is ugly. KaiRo, why do we need a separate #PlacesToolbar ?
Comment 11 neil@parkwaycc.co.uk 2010-08-22 03:46:40 PDT
(In reply to comment #9)
> Well, for one thing, I heard people talk of not being able to turn them off as
> a bug, but OK, I see that side - the rule itself without the space selector is
> superior than the unneededly longer rule in bookmarks.css though :P
Possibly the rule shouldn't apply to the Bookmarks button itself.
Comment 12 neil@parkwaycc.co.uk 2010-08-25 16:22:55 PDT
Comment on attachment 466161 [details] [diff] [review]
Now including bookmarksToolbar.css

> .bookmarks-toolbar-customize {
...
>   display: none;
...
> }
> 
>+toolbarpaletteitem[place] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-customize {
>   display: -moz-box;
> }
...
>+toolbarpaletteitem[place="toolbar"] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-overflow-items,
>+toolbarpaletteitem[place="toolbar"] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-items {
>   visibility: collapse;
> }
> 
>+toolbarpaletteitem[place="palette"] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-overflow-items,
>+toolbarpaletteitem[place="palette"] > #personal-bookmarks > #PlacesToolbar > .bookmarks-toolbar-items {
>   display: none;
> }
These visibility/display rules (and their equivalents in other themes) now have equivalents in content places.css and aren't needed here any more.
Comment 13 Stefan [:stefanh] 2010-08-26 13:24:20 PDT
Created attachment 469588 [details] [diff] [review]
Now without removed rules

Right, these rules are now gone - turns out that the patch was bitrottened.
Comment 14 neil@parkwaycc.co.uk 2010-08-26 17:00:15 PDT
Comment on attachment 469588 [details] [diff] [review]
Now without removed rules

[Note to self: Bookmarks items don't display correctly in text mode, with or without the patch, so must be a separate bug. Sigh.]
Comment 15 Stefan [:stefanh] 2010-08-27 15:45:16 PDT
http://hg.mozilla.org/comm-central/rev/1332ded22012

Note You need to log in before you can comment on or make changes to this bug.