Closed Bug 419540 Opened 12 years ago Closed 2 years ago

menuitem-iconic forcing small icons in FF 3.0 betas

Categories

(Toolkit :: XUL Widgets, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: carglue, Unassigned)

References

Details

(Keywords: addon-compat)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3

Beginning with the Firefox 3 betas, the "menuitem-iconic" class no longer respects the "Use Small Icons" option on the "Customize Toolbar" dialog for the default WinXP winstripe theme.  The icons are always displayed in small 16x16 size, which is the same quirk that used to plague the default MacOSX theme.  However, the "menu-iconic" (not the same) shows both large and small icons just fine.

I've been unable to find any documentation regarding this change (including on http://developer.mozilla.org/en/docs/Theme_changes_in_Firefox_3).

Note: this is not a new code issue, as the same xul code and css overlays have been used in my extension for over 3 years. 

Reproducible: Always
Version: unspecified → Trunk
Could you attach a sample of the code that shows the problem? Even just attaching an entire extension, or linking to one, would help.
Version: Trunk → unspecified
The extension is undergoing heavy rework for FF3 compatibility at the moment and the last public release is currently unusable under FF3.

But if it helps, here are the relevant code lines -

From the .xul Overlay file:
--------------------------
      <toolbaritem id="dcToolbarOptions" flex="0">
         <toolbarbutton label="&dc.options.label;" tooltiptext="&dc.options.tooltip;" id="dcButtonOptions" class="toolbarbutton-1 dcToolbarDejaClick-button" type="menu" phase="target" command="cmd_dc_options">
            <menupopup>
               <menuitem label="&dc.options.configure.label;" tooltiptext="&dc.options.configure.tooltip;" accesskey="&dc.options.configure.key;" id="dcMenuOptionsConfigure" class="menuitem-iconic" phase="target" command="cmd_dc_configure"/>

		...


From the .css Overlay file:
--------------------------
#dcMenuOptionsConfigure {
    list-style-image: url("chrome://dejaclick/skin/images/dc_config.png");
}
toolbar[iconsize="large"] #dcMenuOptionsConfigure {
  list-style-image: url("chrome://dejaclick/skin/images/dc_config.png");
}
toolbar[iconsize="small"] #dcMenuOptionsConfigure {
  list-style-image: url("chrome://dejaclick/skin/images/dc_config_sm.png");
}

#dcButtonOptions {
   -moz-box-orient: vertical !important;
}
#dcButtonOptions > .toolbarbutton-menu-dropmarker{
  height: 3px !important;
}

(Note: the above height override for the dropmarker is also a new quirk for FF3, because without it, the down arrow high under the menu button becomes grossly padded with space around it, which was never an issue before).
This regression affects any addons having menu icons.  Adding the late-compat keyword and requesting review as a FF3 blocker.
Flags: blocking-firefox3?
Keywords: late-compat
(In reply to comment #0)
> the "menuitem-iconic" class no longer respects the "Use Small Icons" option

I don't see why it should. That option is for toolbar buttons, not for menus.

But I also don't understand why your css rules from comment 2 wouldn't work. On which toolbar did you place your button? Have you verified that it has the iconsize attribute and that its value is "small"?
> I don't see why it should. That option is for toolbar buttons, not for menus.
The "Use Small Icons" option has always controlled the size of the icons for <menusitems> tagged with either the "menu-iconic" or "menuitem-iconic" classes.

Ok, I found the culprit.  Its "menu-iconic-icon".

Someone added a new styling rule to chrome://global/skin/menu.css:

.menu-iconic-icon {
  width: 16px;
  height: 16px;
}

...which now forces the icons of every XUL <menuitem> with class="menuitem-iconic" into a 16x16 icon size, regardless of the actual icon size and in defiance of the "Use Small Icons" option under which it is normally controlled.  (Note that "menu-iconic" was not affected.)

This rougue style rule needs to be removed from global menu.css file and we need to also make sure that this same sort of mangling isn't done in the OSX or Linux default skins either, as I believe OSX may have a similar quirk.

Still recommending this as a FF3 blocker, as it breaks styling on existing extensions.
(In reply to comment #5)
> The "Use Small Icons" option has always controlled the size of the icons for
> <menusitems> tagged with either the "menu-iconic" or "menuitem-iconic" classes.

No, that option alone can't do that. You need to explicitly respect the iconsize like you did in comment 2. I don't think we want to support that, since menu icons are normally 16*16px.
Blocks: 363130
> No, that option alone can't do that. You need to explicitly respect the
iconsize...

Not true.  In my style rules, I only added the following rule in FF3b testing just to see if I could make it work properly again:
toolbar[iconsize="large"] #dcMenuOptionsConfigure {
  list-style-image: url("chrome://dejaclick/skin/images/dc_config.png");
}

And all I was doing by using the toolbar[iconsize="small"] rule in my previous code was to switch the target image file (from a 24x24 default image size to the smaller 16x16 one).  There were no forced height/width restrictions in place on menu icons in FF2, so everything worked as it was supposed to.

> I don't think we want to support that, since menu icons are normally 16*16px.
This is also untrue.  What is normal anyway?  That's up to the css styling rules for the specific application.  Forcing icons into a specific size has no place in a global menu.css file inherited by all XUL applications.

Also, remember my comment that the "menu-iconic" class was not affected.  This means that *in the same menu*, the default styling as it exists now will force some menu icons into a bogus 16x16 shoe, while allowing other menu icons (which have submenus) to expand and contract in size (as they should).  Both are trying to respect the "Use Small Icons" toolbar attribute via their css styling rules, but only one has been hacked in menu.css to apply an improper size to its image.  Its unnatural for a core XUL .css file to force an expectation about what the size of an image will be.

Here's a better question.  Why was this hack added to menu.css anyway in FF3?
(In reply to comment #7)
> There were no forced height/width restrictions in
> place on menu icons in FF2, so everything worked as it was supposed to.

Right. There was no default height or width, which of course isn't the same as 'The "Use Small Icons" option has always controlled the size of the icons for <menusitems>.'

> There were no forced height/width restrictions in
> place on menu icons in FF2, so everything worked as it was supposed to.

What we have now isn't a restriction either. You are free to define custom dimensions in your css file.

> > I don't think we want to support that, since menu icons are normally 16*16px.
> This is also untrue.  What is normal anyway?

Because it's common policy for most operating systems.

> Also, remember my comment that the "menu-iconic" class was not affected.

Yes, we should probably fix that.

> Here's a better question.  Why was this hack added to menu.css anyway in FF3?

see bug 363130
Thanks for the pointer.  Moving the discussion to bug 363130.
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 363130
this is not a duplicate of bug 363130
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
(In reply to comment #5)
> This rougue style rule needs to be removed from global menu.css file and we
> need to also make sure that this same sort of mangling isn't done in the OSX or
> Linux default skins either, as I believe OSX may have a similar quirk.

There's no "mangling" involved. The default styling was changed to a more sensible 16x16 pixels, to match platform convention for iconic menu items. You're of course able to override that styling using something like:

.menu-iconic-icon {
  width: auto;
  height: auto;
}

in your application style sheet. I don't think there's a bug to fix in Firefox here.
Component: Menus → XUL Widgets
Flags: blocking-firefox3?
Product: Firefox → Toolkit
QA Contact: menus → xul.widgets
There was, of course, a behavior change, and we should try and document that. Perhaps in the "Firefox 3 theme changes" document?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: dev-doc-needed
Yes, thank you, I already put a temporary style override in place to deal with this breakage.  But as I was pointing out in bug 363130, to force all XUL applications and extension developers to override a Firefox-specific styling rule (and wasting their time hunting down the issue) is bad karma.  Do you see anywhere in the base CSS styling rules for XUL which forces icons in either tree or table list cells to be a fixed 16x16 size?  That would be silly, right?  Same thing here. 

Lastly, I keep hearing this argument for "standard plaform" styling.  That's hogwash too.  XUL is free form.  Menus of all types may appear vitually anywhere in a XUL interface.  And just like tree and list items, menus can also contain all sorts of content in varying shapes and sizes.  There should be no pre-conceived notion of how large an associated image will be in core XUL styling, only in specific applications (such as a Firefox browser).

Foobar this thing up if you want to, but the approach taken in FF2 and earlier was "The Right Thing To Do" and fixing it properly before FF3 launches was would take all of 5 minutes or less.
(In reply to comment #13)
> But as I was pointing out in bug 363130, to force all XUL
> applications and extension developers to override a Firefox-specific styling
> rule

I've already said this in that bug, but I 'll say it again just so that it's clear: there's nothing Firefox-specific about limiting menu icons to 16x16 pixels, by default. 

> (and wasting their time hunting down the issue) is bad karma.  Do you see
> anywhere in the base CSS styling rules for XUL which forces icons in either
> tree or table list cells to be a fixed 16x16 size?  That would be silly,
> right? Same thing here.

No, I don't think it would be silly, and we should make that change for consistency, *if* it avoids toolkit users from having to set the width/height themselves in the majority of cases. We haven't made such changes because it hasn't been demonstrated to be a problem.

> Lastly, I keep hearing this argument for "standard plaform" styling.  That's
> hogwash too.  XUL is free form.  Menus of all types may appear vitually
> anywhere in a XUL interface.  And just like tree and list items, menus can also
> contain all sorts of content in varying shapes and sizes.  There should be no
> pre-conceived notion of how large an associated image will be in core XUL
> styling, only in specific applications (such as a Firefox browser).

Disagree. The majority of the default XUL styling rules exist to match platform conventions. If things really were "free form", as you put it, all the responsibility for looking decent across platforms would be on the app author, and the toolkit would be less useful than it is today. The goal of the default toolkit styles are to make it easy to have a reasonable-looking app without a lot of work. You're of course welcome to override the default styling with your own if your needs aren't met by the default styling.
Pretty sure this no longer applies.
Status: NEW → RESOLVED
Closed: 12 years ago2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.