Closed
Bug 361096
Opened 18 years ago
Closed 18 years ago
sanitize [w|p]instripe help.css
Categories
(SeaMonkey :: Help Viewer, defect)
SeaMonkey
Help Viewer
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: kairo, Assigned: kairo)
References
Details
Attachments
(1 file, 1 obsolete file)
In bug 360109 comment #4, Neil said:
Mano made a pig's ear of help.css in bug 353673, in particular the style rule
#help-forward-button:not([disabled="true"]):active is wrong for two reasons.
I'll file a patch here to correct it, so that I don't need to use the same style or some magically more specific rules to override those from help.css in an overlay.
Assignee | ||
Comment 1•18 years ago
|
||
This should do it for both themes. The rules are the same, I'm also removing some trailing spaces in the winstripe one, and synching the pinstripe part with the winstripe one.
Requesting review from both jwalden (default owner of help viewer bugs) and Neil (who originally commented that the rules are wrong).
Assignee: jwalden+fxhelp → kairo
Status: NEW → ASSIGNED
Attachment #245872 -
Flags: second-review?(neil)
Attachment #245872 -
Flags: first-review?(jwalden+fxhelp)
Assignee | ||
Comment 2•18 years ago
|
||
Comment on attachment 245872 [details] [diff] [review]
remove the :not() stuff for both [p|w]instripe help.css files
>Index: mozilla/toolkit/themes/pinstripe/help/help.css
>===================================================================
>+#help-print-button:not([disabled="true"]):hover {
>+ -moz-image-region: rect(24px 96px 48px 72px);
>+}
>+
>+#help-print-button:not([disabled="true"]):hover:active {
>+ -moz-image-region: rect(72px 96px 96px 72px);
>+}
>Index: mozilla/toolkit/themes/winstripe/help/help.css
>===================================================================
>-#help-print-button:not([disabled="true"]):hover {
>- -moz-image-region: rect(24px 96px 48px 72px);
>+#help-print-button:not([disabled="true"]):hover {
>+ -moz-image-region: rect(24px 96px 48px 72px);
> }
>
>-#help-print-button[disabled="true"] {
>- -moz-image-region: rect(48px 96px 72px 72px);
>+#help-print-button:not([disabled="true"]):hover:active {
>+ -moz-image-region: rect(72px 96px 96px 72px);
> }
Sorry, the |:not([disabled="true"])| part should be removed there as well. Done locally, please do reviews as if that would have been removed in the patch already.
Comment 3•18 years ago
|
||
Would be nice to know why it's considered wrong...
Assignee | ||
Comment 4•18 years ago
|
||
OK, per some ICQ talk, the :not() is probably OK, but some other sanitizing of those files would be good, patch upcoming
Summary: remove :not([disabled="true"]) from [w|p]instripe help.css → sanitize [w|p]instripe help.css
Assignee | ||
Comment 5•18 years ago
|
||
Here's the patch, using #HelpToolbar for all references to the toolbar, :hover:active consistently for active buttons, and using display instead of visibility to hide #context-copy
And yes, I know #developers is an IRC channel and not ICQ, somehow my mind got confused...
Attachment #245872 -
Attachment is obsolete: true
Attachment #246805 -
Flags: second-review?(neil)
Attachment #246805 -
Flags: first-review?(mano)
Attachment #245872 -
Flags: second-review?(neil)
Attachment #245872 -
Flags: first-review?(jwalden+fxhelp)
Updated•18 years ago
|
Attachment #246805 -
Flags: second-review?(neil) → second-review+
Comment 6•18 years ago
|
||
Comment on attachment 246805 [details] [diff] [review]
patch, v2: #HelpToolbar, :hover:active and menuitem collapse cleanup
>Index: mozilla/toolkit/themes/pinstripe/help/help.css
>===================================================================
> @import url("chrome://global/skin/");
> @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
>
>-toolbar#HelpToolbar{
>+#HelpToolbar{
please add a space before the brace while you're here.
> /* Hide labels for the toolbar because we really don't need them what with the
> tooltips */
>-toolbar#HelpToolbar .toolbarbutton-text{
>+#HelpToolbar .toolbarbutton-text{
> display: none;
> }
ditto.
>
> /* Style the back/forward dropmarks to connect them to the buttons */
>
> /* affects where the dropmark will appear */
>-toolbar#HelpToolbar .toolbarbutton-menubutton-stack {
>+#HelpToolbar .toolbarbutton-menubutton-stack {
> margin: 0px !important;
> padding: 0px;
> -moz-box-orient: horizontal;
> }
>
Just remove this selector, it applies to nothing.
r=mano otherwise.
Attachment #246805 -
Flags: first-review?(mano) → first-review+
Assignee | ||
Comment 7•18 years ago
|
||
Sorry for the check-in lag, been too busy lately.
Checked in now, this should be FIXED.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Product: Toolkit → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•