Getting denied by AMO because of CSS name changes...

[patrickjdempsey]

Just got this letter from a reviewer testing on Firefox 3.6:

Comments:
This version didn't pass full review because of the following issues:

1) This seems to break the styling of type=checkbox toolbarbuttons like the ones used in Console². They always appear to be unchecked.

2) I'm seeing quite a few CSS warnings with this:

Warning: Expected end of value but found 'border-radius'. Error in parsing value for 'margin'. Declaration dropped.
Source file: chrome://browser/skin/titlebar.css
Line: 10
----------
Warning: Error in parsing value for '-moz-appearance'. Declaration dropped.
Source file: chrome://browser/skin/titlebar.css
Line: 68
----------
Warning: Unknown pseudo-class or pseudo-element '-moz-window-inactive'. Ruleset ignored due to bad selector.
Source file: chrome://browser/skin/titlebar.css
Line: 87
----------
Warning: Unknown pseudo-class or pseudo-element '-moz-window-inactive'. Ruleset ignored due to bad selector.
Source file: chrome://browser/skin/titlebar.css
Line: 123
----------
Warning: Unknown property '-moz-transition'. Declaration dropped.
Source file: chrome://browser/skin/tabbrowser/tabbrowser.css
Line: 98
----------
Warning: Unknown pseudo-class or pseudo-element '-moz-placeholder'. Ruleset ignored due to bad selector.
Source file: chrome://global/skin/textbox.css
Line: 85
----------
Warning: Unknown pseudo-class or pseudo-element '-moz-window-inactive'. Ruleset ignored due to bad selector.
Source file: chrome://global/skin/menu.css
Line: 69

Also, Firefox 4 will be released very soon. If you intend to make your add-on compatible, the time is now.

You need to correct them to get full approval. Thanks.

Can anyone else here see the irony? Why does this happen with *every* version update? I guess AMO doesn't provide the reviewers with a list of new property and pseudo-element names that are needed for next version compatibility. Also, this isn't the first time I've been denied because of Console2, they do some weird things that almost always breaks my themes... sigh. I wrote the reviewer a response, we shall see what they have to say.

[patrickjdempsey]

______, all of the Errors you are seeing are compatibility errors due to making things compatible with Firefox 4.0. These are due to changes in the names of CSS properties and the addition of new element selectors that *required* to make themes compatible with Firefox 4.0. I expect every theme that will be attempting to create a version compatible with both 3.6 and 4.0 will have similar errors. *Everything* in titlebar.css for instance belongs to elements that don't even exist in the 3.6 DOM so they shouldn't have any actual impact on 3.6.

It's possible to use version-specific CSS in skins if you use a chrome.manifest.

You've got to be kidding me.

[Philip Chee]

Also, this isn't the first time I've been denied because of Console2, they do some weird things that almost always breaks my themes... sigh. I wrote the reviewer a response, we shall see what they have to say.

MCDavis worked with me to get Console2 more themer friendly. But I've never received any email about your theme issues with Console2.

Phil

[patrickjdempsey]

I figured out what it is.... it's because of all of the checkbox type toolbarbuttons, I had them unstyled. In Firefox itself there aren't any except for the .toolbarbutton-1 checkboxes (History and Bookmarks sidebar). I've run into that problem before and I keep forgetting it! Sorry if I offended, I'm just sorta frustrated with getting denied over Warnings in the Error Console that are exactly like what you see in the Error Console when you visit ANY website that features compatibility hacks for different browsers / browser versions.

[mcdavis]

I expect every theme that will be attempting to create a version compatible with both 3.6 and 4.0 will have similar errors.

Absolutely correct, without resorting to some rather extreme contortions in chrome.manifest.

The problem with using a chrome.manifest approach is that the app-version-specific rules are so widely distributed throughout the theme that addressing all of them would basically mean including two versions of the theme -- one for Fx3.6, one for Fx4 -- in the same jar. This would inflict an increase in download size for no meaningful benefit to the end user, most of whom never ever (ever) look at error messages.

And, with four more versions of Firefox rolling out over the next year, there's a good chance this'll be an issue again and again.

What's the real harm caused by the presence of app-version-specific error messages? Depending on how many messages, it could make it hard for some developers to notice other, more important errors, but there are ways around that for that small subgroup of users. Other than that, no harm that I can see.

Everybody agrees it's better to have fewer error messages rather than more, and errors which can be fixed should be, but Firefox's implementation makes strict compatibility impossible without making end users pay a price. I think error messages arising from version-specific differences that don't represent something actually breaking should be overlooked.

In other words, there's a difference between an error message and an error.

[patrickjdempsey]

The thing is, I just *started* adding a few Firefox 4.0 stuff into this one theme. I just wanted to get the 3.6 version up since I had basically finished all of the 3.6-centric changes I was making. No sense in waiting to have the 4.0 stuff finished to wait on other fixes right? So When I get all of the support stuff for Tab Groups, the new Addons Manager, Doorhanger notifications, etc etc I'll expect more and more Errors. And of course, all of those wonderful theme-detection selectors you documented that I want to use in another theme will fail the 3.6 test as well... as well as Glass support selectors I'm betting. Maybe not all reviewers will be this strict, and it's possible that if your theme is 4.0 compatible that it may never even be reviewed on 3.6... and of course, no Error Console problems in 4.0.

[mcdavis]

Yeah man, I'm right there with you Patrick. I use

Code: Select all

-moz-background-clip: padding;
background-clip: padding-box;

and things like that throughout the theme, and as we all know there's no way to write that to work with both Fx3.6 and Fx4 without error messages. If I fixed that with chrome.manifest it would balloon the theme size to around 3.nMB, for large values of n. I don't see a large theme as a good outcome, and I don't see any other choice. (We can't replace individual files in chrome.manifest, right? Just entire skin providers eg all of browser or all of global?) So, there's got a be a compromise somewhere, and the greatest good is done by just not worrying about those error messages.

We've seen this coming from the day bug 549809 landed (comment 11), and now it's here.

Maybe not all reviewers will be this strict, and it's possible that if your theme is 4.0 compatible that it may never even be reviewed on 3.6

Hoping!

[ShareBird]

(We can't replace individual files in chrome.manifest, right? Just entire skin providers eg all of browser or all of global?)

viewtopic.php?f=18&t=906535

[patrickjdempsey]

Ahhhhh.... I read that before a few years ago and didn't comprehend it. This would also be a really good way of dealing with extensions.css which is completely different in 3.6 than in 4.0.... you could have it blank with one line importing the file from a compatibility directory. Same with my titlebar.css... for 3.6 it could just be blank. I'll look at this more in-depth tonight.

[patrickjdempsey]

Hey man, I hope it's OK but I've linked that thread to Jorge's AMO blog about 4.0 compatibility:
http://blog.mozilla.com/addons/2010/11/ ... firefox-4/

[patrickjdempsey]

Hey man, I hope it's OK but I've linked that thread to Jorge's AMO blog about 4.0 compatibility:
http://blog.mozilla.com/addons/2010/11/ ... firefox-4/

[ShareBird]

Of course it's ok!

[patrickjdempsey]

A note for folks trying this method. I've noticed that many of the CSS files in the global skin will not allow you to use @import for some reason. But putting the @import's for those in browser.css or the main global.css seems to work fine.

[laura_abc]

I've also never got mails about this, why?