361096 - sanitize [w|p]instripe help.css

Status: RESOLVED FIXED

(SeaMonkey :: Help Viewer, defect)

Description

[Robert Kaiser]

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.

Comment 1

[Robert Kaiser]

Attachment: remove the :not() stuff for both [p|w]instripe help.css files

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).

Comment 2

[Robert Kaiser]

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

[Asaf Romano (gone)]

Would be nice to know why it's considered wrong...

Comment 4

[Robert Kaiser]

OK, per some ICQ talk, the :not() is probably OK, but some other sanitizing of those files would be good, patch upcoming

Comment 5

[Robert Kaiser]

Attachment: patch, v2: #HelpToolbar, :hover:active and menuitem collapse cleanup

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...

Comment 6

[Asaf Romano (gone)]

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.

Comment 7

[Robert Kaiser]

Sorry for the check-in lag, been too busy lately.

Checked in now, this should be FIXED.