Add-ons/Reviewers/Guide/CompleteThemes: Difference between revisions
(Complete themes :() |
(Done with feature testing, on to identity boxes) |
||
Line 4: | Line 4: | ||
Testing should be done on a new profile. The code validator doesn't have many useful tests for themes, but it should flag if there's any code in the theme. Reject if that's the case. | Testing should be done on a new profile. The code validator doesn't have many useful tests for themes, but it should flag if there's any code in the theme. Reject if that's the case. | ||
While reviewing themes, watch the Browser Console console for any CSS warnings that are flagged for chrome paths containing the word "skin". Warnings should be copied from the error console and pasted into review notes for the developer to investigate. Generally, warnings about CSS issues should '''not''' be grounds for withholding a full review of a theme. However, providing notes to developers about CSS issues can be tremendously helpful to theme development. | |||
{| cellspacing="0" cellpadding="1" border="0" style="width: 80%;" | {| cellspacing="0" cellpadding="1" border="0" style="width: 80%;" | ||
Line 29: | Line 31: | ||
|} | |} | ||
== Test Pages | == Feature testing == | ||
{| cellspacing="0" cellpadding="1" border="0" style="width: 80%;" | |||
|+ | |||
|- | |||
! style="border-bottom: 2px solid black" scope="col" | Feature | |||
! style="border-bottom: 2px solid black" scope="col" | Action on failure | |||
! style="border-bottom: 2px solid black" scope="col" | What to test | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Edit > Find | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure find toolbar is styled appropriately and functions correctly. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | View > Toolbars > Customize | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure the customization UI is formatted reasonably. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | View > Toolbars > Bookmarks Toolbar | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure toolbar is formatted reasonably. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | View > Sidebar > Bookmarks and View > Sidebar > History | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure sidebar is formatted appropriately. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | History > Show All History | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure history/bookmarks library is formatted appropriately. Drill in to a specific bookmark/history item. Test drop down menus and forward/back buttons. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Tools > Set Up Sync | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Work through the sync dialog as if you are creating a new account and connecting to a device (completion of account creation and connecting not necessary). Make sure Sync is formatted appropriately and things seem to work. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preferences / Options window | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure all tabs are formatted and functioning correctly. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Help > About | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure it is styled appropriately and must use an appropriate Firefox logo. | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Web Developer Tools | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add note | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Make sure the all tools open correctly and their UI is usable. Note that the developer tools aren't required to be styled by a theme. | |||
|} | |||
== Identity Boxes == | |||
Identity boxes are an important user facing security feature that must be carefully inspected to make sure they function properly. For each style of identity box, make sure to click on the identity box icon to make sure the drop down door hanger is styled reasonably and contains the appropriate icons. | |||
{| cellspacing="0" cellpadding="1" border="0" style="width: 80%;" | |||
|+ | |||
|- | |||
! style="border-bottom: 2px solid black" scope="col" | Security Level | |||
! style="border-bottom: 2px solid black" scope="col" | What to test | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | '''HTTPS Extended Identity''' | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <ul><li>Test URL: [https://addons.mozilla.org/ https://addons.mozilla.org/] | |||
<li>Default background color: Very Light Green | |||
<li>Default text and border color: Dark Green | |||
<li>Identity box text: Company Name | |||
<li>Icon: Padlock | |||
<li>Requirement: Background, text and/or border colors must be unique from other identity boxes. Must use padlock icon. Must sufficiently stand out from rest of theme. Using the color green is strongly recommended. | |||
<li>Sample screen capture:<br>[[Image:IdentityBoxFF14-ExtendedValidation.png]] | |||
</ul> | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | '''HTTPS Basic Identity''' | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <ul><li>Test URL: [https://bugzilla.mozilla.org/ https://bugzilla.mozilla.org/] | |||
<li>Default background color: White | |||
<li>Default text and border color: Dark Grey | |||
<li>Identity box text: None | |||
<li>Icon: Padlock | |||
<li>Requirement: Must utilize a padlock icon. | |||
<li>Sample screen capture:<br>[[Image:IdentityBoxFF14-BasicValidation.png]] | |||
</ul> | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | '''HTTPS Mixed Content''' | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <ul><li>Test URL: [https://people.mozilla.com/~bsterne/tests/62178/test.html https://people.mozilla.com/~bsterne/tests/62178/test.html] | |||
<li>Default background color: light grey | |||
<li>Identity box text: none | |||
<li>Icon: Globe | |||
<li>Requirement: Must NOT use secured padlock icon (broken padlock okay). | |||
<li>Sample screen capture:<br>[[Image:IdentityBoxFF14-MixedContent.png]] | |||
</ul> | |||
|- style="vertical-align: top;" | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | '''HTTP Unsecured Connection''' | |||
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <ul><li>Test URL: [http://www.mozilla.org http://www.mozilla.org] | |||
<li>Default background color: light grey | |||
<li>Identity box text: none | |||
<li>Icon: Globe | |||
<li>Requirement: Background color must be different from HTTPS extended identity box, must NOT use padlock icon of any sort. | |||
<li>Sample screen capture:<br>[[Image:IdentityBoxFF14-HTTP.png]] | |||
</ul> | |||
|} | |||
= Test Pages = | |||
All of the following URLs should be tested. | All of the following URLs should be tested. | ||
Line 172: | Line 269: | ||
====Special Considerations==== | ====Special Considerations==== | ||
Windows and OS-X use different CSS rules to generate OS native scrollbars. As a result any cross OS theme either needs to target different scrollbar rules for each OS or use custom XUL scrollbars. | Windows and OS-X use different CSS rules to generate OS native scrollbars. As a result any cross OS theme either needs to target different scrollbar rules for each OS or use custom XUL scrollbars. | ||
Revision as of 21:46, 21 January 2015
Complete themes are less security-sensitive than other add-ons, so it doesn't take much for a theme to pass preliminary review. However, to obtain full review, lots of attention to detail is required in the review. There's also the special case of multi-package installers, which are generally used to bundle a theme and a configuration extension. In this case both an extension review and an add-on review are necessary.
Basic testing
Testing should be done on a new profile. The code validator doesn't have many useful tests for themes, but it should flag if there's any code in the theme. Reject if that's the case.
While reviewing themes, watch the Browser Console console for any CSS warnings that are flagged for chrome paths containing the word "skin". Warnings should be copied from the error console and pasted into review notes for the developer to investigate. Generally, warnings about CSS issues should not be grounds for withholding a full review of a theme. However, providing notes to developers about CSS issues can be tremendously helpful to theme development.
Issue | Action | Notes |
---|---|---|
Text and/or buttons are difficult to read. | Preliminary review | |
Theme has an inconsistent look. | Preliminary review | You should avoid judging a theme for thinking it looks ugly, since it is a very subjective metric. However, themes should be consistent about their look. |
UI elements don't fit within their boundaries and appear "clipped". | Add note | This is relevant for menus and dialogs. If anything's clipped and/or if resizing is required to see it completely, make a note of it and request the author to address the issue. |
No preview image or unsuitable preview image. | Add note |
Feature testing
Feature | Action on failure | What to test |
---|---|---|
Edit > Find | Preliminary review | Make sure find toolbar is styled appropriately and functions correctly. |
View > Toolbars > Customize | Preliminary review | Make sure the customization UI is formatted reasonably. |
View > Toolbars > Bookmarks Toolbar | Preliminary review | Make sure toolbar is formatted reasonably. |
View > Sidebar > Bookmarks and View > Sidebar > History | Preliminary review | Make sure sidebar is formatted appropriately. |
History > Show All History | Preliminary review | Make sure history/bookmarks library is formatted appropriately. Drill in to a specific bookmark/history item. Test drop down menus and forward/back buttons. |
Tools > Set Up Sync | Preliminary review | Work through the sync dialog as if you are creating a new account and connecting to a device (completion of account creation and connecting not necessary). Make sure Sync is formatted appropriately and things seem to work. |
Preferences / Options window | Preliminary review | Make sure all tabs are formatted and functioning correctly. |
Help > About | Preliminary review | Make sure it is styled appropriately and must use an appropriate Firefox logo. |
Web Developer Tools | Add note | Make sure the all tools open correctly and their UI is usable. Note that the developer tools aren't required to be styled by a theme. |
Identity Boxes
Identity boxes are an important user facing security feature that must be carefully inspected to make sure they function properly. For each style of identity box, make sure to click on the identity box icon to make sure the drop down door hanger is styled reasonably and contains the appropriate icons.
Security Level | What to test |
---|---|
HTTPS Extended Identity |
|
HTTPS Basic Identity |
|
HTTPS Mixed Content |
|
HTTP Unsecured Connection |
|
Test Pages
All of the following URLs should be tested.
URL | Policy on failure | What to test |
---|---|---|
about:addons |
|
|
about:config | Add Note | Make sure it is formatted correctly. While in about config, set the following keys to aid later testing:
|
about:permissions | Add Note | Make sure it is formatted correctly. While in about:permissions, make sure that if there are entries for them, the following domains have the necessary permissions:
|
AMO Addon |
|
|
Mozilla's Mission |
|
|
Google PageSpeed extension download | Preliminary Review | On page click on "install page speed link and make sure a prompt appears in identity box verifying whether or not to install extension. |
Mozilla geolocation Test | Preliminary Review | Click on link labeled "Give it a try" and then the "where am I" button to activate geolocation alert. Make sure it displays correctly. |
Mozilla phishing test page | Preliminary Review | Phishing warning test page. Check notification strip after ignoring phishing warning. To reset warning go to history menu and open "show all history" then right mouse click on this page's entry and select "forget about this site". |
Mozilla malware test page | Preliminary Review | Malware warning page. Check notification strip after ignoring malware warning. To reset warning go to history menu and open "show all history" then right mouse click on this page's entry and select "forget about this site". |
https://63.245.217.20/ | Preliminary Review | Untrusted connection alert. Click on identity box to test drop down details. |
jar:https://bugzilla.mozilla.org/attachment.cgi?id=288634!/ | Preliminary Review | Unsafe Remote File Type. Access denied. Make sure it is formatted reasonably. |
Mozilla FTP Archive | Preliminary Review | FTP. Make sure it is formatted reasonably. |
chrome://global/content/bindings/textbox.xml | Add Note | Unformatted XML Preview. Make sure it is formatted reasonably. |
Additional "about" pages. | Add Note | Make sure the following "about" pages are formatted reasonably:
|
Usability Tests
The Themes should be usable without Mouse - widgets have to change their appearance when tabbed through them with the keyboard. (e.g. checkboxes, radio buttons, tabs, should indicate key focus). Many of these tests can be conducted in conjunction with other tests above.
- Tab Strip: Tab through tab strip using Ctrl+Tab. Active tab should be visually different from other tabs.
- Group Tabs: Activate group tabs using Ctrl+Shift+E the highlighted tab should be visually different from other tabs use the up/down/left/right arrows to navigate from tab to tab and then press enter to open the desired tab.
- Menu Bar: Activate the file menu using Alt+F and then use keyboard to navigate through menus and sub-menus. The selected menu item must be visually different and the highlighted text must be legible.
- Places: In the places (History/Bookmarks) panel tab around and use keyboard arrows to navigate. Make sure it is visible what the selected item is.
- Options: Open the options panel and make sure you can navigate around the options panel and that you know what has focus. You must be able to change tabs, and settings from the keyboard.
OS Specific Issues
Linux
- Unstyled Firefox Button: On Linux, themes sometimes have improperly or totally unstyled Firefox button, and/or the Firefox button not changing styling when in private browsing mode. The primary cause of this issue is that the Firefox button has a different ID in Linux from Windows. As a result, in the browser.css file any reference to #appmenu-button also needs a comparable reference to #appmenu-toolbar-button.
- Strong Black borders Around URL Bar Door Hangers (drop down panels): Many Windows based themes make use of the box-shadow style rule, however, it is not supported on Linux Firefox and resulting in the strong black border. If the strong black border is noticed, it should be noted to the developer and they should be encouraged to make sure box-shadow style rules only target Windows.
OS-X
If you test any theme that is supposed to support OS-X please make sure that if the following files exist in "chrome://browser/skin/" they were also copied to "chrome://browser/skin/lion/":
- keyhole-circle.png
- Toolbar.png
- toolbarbutton-dropmarker.png
- tabbrowser/alltabs-box-bkgnd-icon.png
- tabview/tabview.png
- places/toolbar.png
The other solution theme developers can do is rename the files to something non-default and then make the proper file mappings in the theme's CSS files. NOTE: This issue doesn't require OS-X to test, you only need to look in the theme contents (via the "contents" or "compare" links) to see if the files in question exist. For more information see Bug 679708 and Bug 702558
Win7 w/Aero Glass Support
Win7 doesn't always go into full screen properly
Any theme that supports Aero Glass transparency needs to be tested going from a normal window to full screen. Often times when this is tried, the browser window will disappear or not go to full screen properly. When this happens, developers need to be provided the following note. See Bug 732757 and this MozillaZine thread for more information.
On Win7 with Aero Glass support Firefox doesn't always go to full screen mode from a normal window properly. The resolution to this issue is to add the following code to your browser.css file somewhere below where the main-window is made transparent to support Aero glass. For more information about this issue please see https://bugzilla.mozilla.org/show_bug.cgi?id=732757 and http://forums.mozillazine.org/viewtopic.php?f=18&t=2438459. ++++++++++ @media all and (-moz-windows-compositor) { /* Make transition to Fullscreen mode seamlessly in Firefox 10+ */ #main-window[inFullscreen="true"] { -moz-appearance: none; background-color: -moz-dialog!important; } } +++++++++
Win7 sometimes missing min/max/restore/close button group when title bar disabled
On Win7 Aero, when tabs are on top and the menu bar is disabled, some themes will be missing the min/max/restore/close button on the right side of the title bar. When this happens please point the theme developer to the following MozilliaZine forum threads for the solution to this issue: http://forums.mozillazine.org/viewtopic.php?f=18&t=2131121 http://forums.mozillazine.org/viewtopic.php?f=18&t=1953371&start=60
Special Considerations
Windows and OS-X use different CSS rules to generate OS native scrollbars. As a result any cross OS theme either needs to target different scrollbar rules for each OS or use custom XUL scrollbars.