Tidy up CSS after Bug 428216

RESOLVED FIXED in seamonkey2.0a3

Status

--
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: philip.chee, Assigned: philip.chee)

Tracking

Trunk
seamonkey2.0a3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 353808 [details] [diff] [review]
Patch v1.0 shuffle stuff

In order to make reviewing the patch in Bug 428216 easier I moved several styles to the ends of navigator.css. This patch moves the styles to a more reasonable position. In the attached patch I have also included a fix for Bug 470034.
Attachment #353808 - Flags: review?(neil)
(Assignee)

Comment 1

10 years ago
> Created an attachment (id=353808) [details]
> Patch v1.0 shuffle stuff

-/* Prevent [mode="icons"|"text"] from hiding the label and icon */
-#bookmarks-ptf .bookmark-item > .toolbarbutton-text,
-#bookmarks-ptf .bookmark-item > .toolbarbutton-icon {
+/* Prevent [mode="icons"] from hiding the label */
+#bookmarks-ptf .bookmark-item > .toolbarbutton-text {
   display: -moz-box !important;
+}

Initially I followed Firefox here. Historically XPFE the personal toolbar and the toolbar bookmark items were never configurable. The patch in Bug 428216 maintained this immutability. This patch allows the icon to be turned off/on according to the mode setting of the toolbar however the labels are still always forced on. The reasoning is as follows:

The bookmark items may never have any (fav)icons if the favicons and site_icons preferences are turned off (the default). Hiding the labels would just shows a row of identical "bookmark" icons and would probably cause some consternation.

turning off the icons is safe in that the bookmark items (should) always have distinguishable text.
(Assignee)

Comment 2

10 years ago
There is also a slight inconsistency in the "bookmarks-button" button that I haven't addressed yet and am unsure if it does need to be adjusted. This button has a class of "bookmark-item" rather than "toolbarbutton-1 chromeclass-toolbar-additional". This means that:

1. It is styled as a bookmark item with the label to the side of the icon, and
2. It does not respond to the toolbar mode and label position settings.

Should I change the class to toolbarbutton-1 so that it can react to the toolbar settings?

Comment 3

10 years ago
Comment on attachment 353808 [details] [diff] [review]
Patch v1.0 shuffle stuff

r=me except on the bookmark icon changes - I think we need to be able to differentiate between folders and bookmarks.
Attachment #353808 - Flags: review?(neil) → review+

Comment 4

10 years ago
(In reply to comment #2)
> There is also a slight inconsistency in the "bookmarks-button" button that I
> haven't addressed yet and am unsure if it does need to be adjusted. This button
> has a class of "bookmark-item" rather than "toolbarbutton-1
> chromeclass-toolbar-additional".
Presumably this is to allow it to be dragged separately? I think we may have to add extra "fixup" rules like we do for the Home button in that case.
(Assignee)

Comment 5

10 years ago
Created attachment 353839 [details] [diff] [review]
Patch v1.1 *only* shuffle stuff

Carrying forward r+, requesting sr+

Shuffle only changes, leaving out the bookmark item changes.
Attachment #353808 - Attachment is obsolete: true
Attachment #353839 - Flags: superreview?(neil)
Attachment #353839 - Flags: review+
(Assignee)

Comment 6

10 years ago
Comment on attachment 353839 [details] [diff] [review]
Patch v1.1 *only* shuffle stuff

No UI changes so no sr required (I think)
Attachment #353839 - Flags: superreview?(neil)

Comment 7

10 years ago
Pushed changeset 7af00ea1e7c2 to comm-central.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0a3
(Assignee)

Updated

10 years ago
Blocks: 459509
You need to log in before you can comment on or make changes to this bug.