Firefox/Projects/TabCandy/Work: Difference between revisions

no edit summary
No edit summary
 
(26 intermediate revisions by 3 users not shown)
Line 8: Line 8:
* [http://raysquare.com/ Raymond Lee], [http://twitter.com/raymondlee/ @raymondlee], IRC: raymondlee
* [http://raysquare.com/ Raymond Lee], [http://twitter.com/raymondlee/ @raymondlee], IRC: raymondlee
* [http://seanedunn.com/ Sean Dunn], [http://twitter.com/somenotes/ @somenotes], IRC: seandunn
* [http://seanedunn.com/ Sean Dunn], [http://twitter.com/somenotes/ @somenotes], IRC: seandunn
* [http://kevinhanes.net/ Kevin Hanes], [http://twitter.com/mr_yuk/ @mr_yuk]
* [http://kevinhanes.net/ Kevin Hanes], [http://twitter.com/mr_yuk/ @mr_yuk], IRC: kevinh
* [http://www.digitale-enthusiasten.de/ Tim Taubert], [http://twitter.com/ttaubert/ @ttaubert], IRC: ttaubert


= Important Pages =
= Important Pages =
Line 18: Line 19:
* [[Firefox/Projects/Tabcandy/CodeGuide|Reviewers Guide]]
* [[Firefox/Projects/Tabcandy/CodeGuide|Reviewers Guide]]
* [http://azarask.in/projects/tabcandy/review.html Panorama Review Dashboard]
* [http://azarask.in/projects/tabcandy/review.html Panorama Review Dashboard]
* [[Firefox/Projects/Tabcandy/Panorama 1 Debrief|Panorama 1 Debrief]]


= Design =
= Design =
Line 28: Line 30:
* [[Firefox/Projects/TabCandy/Design/UndoCloseGroup|Undo Close Group]]
* [[Firefox/Projects/TabCandy/Design/UndoCloseGroup|Undo Close Group]]
* [http://www.flickr.com/photos/azaraskin/4946904112/ App tabs]
* [http://www.flickr.com/photos/azaraskin/4946904112/ App tabs]
* [[Firefox/Projects/TabCandy/Design/MiniGroups|MiniGroups]]
* [[Firefox/Projects/TabCandy/Design/Bookmarks|Bookmarks and History in Panorama]]
* [[Firefox/Projects/TabCandy/Design/Next|What's next for Panorama]]


= Bugs =  
= Bugs =  
Line 33: Line 38:
Useful bug queries:  
Useful bug queries:  


* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&field0-0-0=blocked&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=anywordssubstr&value0-0-0=597043%2C%20597763 beta8]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&field0-0-0=blocked&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=anywordssubstr&value0-0-0=627096 beta11]
* [https://bugzilla.mozilla.org/buglist.cgi?quicksearch=blocked%3A597043+first+bug beta8 "good first bug"]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&field0-0-0=blocked&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=anywordssubstr&value0-0-0=585689 Final 1.0 (Firefox 4.0 RC)]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&field0-0-0=blocked&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=anywordssubstr&value0-0-0=598154 beta9]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&field0-0-0=blocked&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=anywordssubstr&value0-0-0=585689 Final 1.0 (Firefox 4.0)]
* [https://bugzilla.mozilla.org/buglist.cgi?negate0=1&field0-0-0=blocked&type0-0-1=equals&field0-0-1=target_milestone&resolution=---&query_format=advanced&value0-0-1=Future&type0-0-0=anyexact&value0-0-0=597043%2C%20597763%2C%20598154%2C%20585689&component=TabCandy&product=Firefox not punted, but not assigned to a milestone]
* [https://bugzilla.mozilla.org/buglist.cgi?negate0=1&field0-0-0=blocked&type0-0-1=equals&field0-0-1=target_milestone&resolution=---&query_format=advanced&value0-0-1=Future&type0-0-0=anyexact&value0-0-0=597043%2C%20597763%2C%20598154%2C%20585689&component=TabCandy&product=Firefox not punted, but not assigned to a milestone]
* [https://bugzilla.mozilla.org/buglist.cgi?priority=--&field0-0-0=target_milestone&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=notequals&value0-0-0=Future&component=TabCandy&product=Firefox fresh bugs (haven't been punted or prioritized)]
* [https://bugzilla.mozilla.org/buglist.cgi?priority=--&field0-0-0=target_milestone&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=notequals&value0-0-0=Future&component=TabCandy&product=Firefox fresh bugs (haven't been punted or prioritized)]
* [https://bugzilla.mozilla.org/buglist.cgi?keywords=perf&keywords_type=allwords&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&component=TabCandy&product=Firefox perf bugs]
* [https://bugzilla.mozilla.org/buglist.cgi?field0-0-0=cf_blocking_20&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=nowordssubstr&value0-0-0=---%2C%20-%2C%20%3F&component=TabCandy&product=Firefox blocking]
* [https://bugzilla.mozilla.org/buglist.cgi?field0-0-0=target_milestone&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&type0-0-0=notequals&value0-0-0=Future&component=TabCandy&product=Firefox all non-punted bugs]


Also, quick search is your friend. There's an [http://people.mozilla.org/~johnath/bugzilla/BeltznerDoesQuicksearch.ogv intro screen-cast] to get you started, and a [http://www.squarefree.com/bugzilla/quicksearch-help.html reference] for all the codes you can use.
Also, quick search is your friend. There's an [http://people.mozilla.org/~johnath/bugzilla/BeltznerDoesQuicksearch.ogv intro screen-cast] to get you started, and a [http://www.squarefree.com/bugzilla/quicksearch-help.html reference] for all the codes you can use.
If you see a bug assigned to someone but with a status of "NEW" (as opposed to "ASSIGNED"), they're probably not actually working on it... feel free to grab it for yourself (and update the status of course).


= Architecture =  
= Architecture =  
Line 52: Line 60:


= Development =  
= Development =  
There's a variety of useful info over at the [https://developer.mozilla.org/En/Developer_Guide MDC Developer Guide].


''These instructions assume you're on a Mac; please update with info for other platforms as needed.''
''These instructions assume you're on a Mac; please update with info for other platforms as needed.''
Line 90: Line 100:
Here are some troubleshooting tips:
Here are some troubleshooting tips:
* If your test spawns a window, you may notice that you get unexpected failures in the private browsing tests even if you properly clean it up. This is caused by unresolved closed windows in the session store. To remove this symptom from affecting later tests, use code such as that shown in this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=475838&action=diff to clear the session store's record of closed windows before calling finish().
* If your test spawns a window, you may notice that you get unexpected failures in the private browsing tests even if you properly clean it up. This is caused by unresolved closed windows in the session store. To remove this symptom from affecting later tests, use code such as that shown in this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=475838&action=diff to clear the session store's record of closed windows before calling finish().
= Tools =
* [https://developer.mozilla.org/en/Force_RTL Force RTL] - for testing "right to left" orientation on "left to right" (e.g. English) systems
* [https://addons.mozilla.org/en-US/firefox/downloads/file/107284/firefox-framerate-monitor-1.0-fx+fn+tb.xpi Frame Rate Monitor] - for analyzing animation speed
* [https://developer.mozilla.org/En/DOM_Inspector DOM Inspector] - works even for chrome such as chrome://browser/content/tabview.html
* [http://getfirebug.com/wiki/index.php/Chromebug_User_Guide Chromebug] - Firebug for chrome


= Mercurial Queues =
= Mercurial Queues =
Line 106: Line 123:


= Landing =  
= Landing =  
Also see [https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch How to Submit a Patch] on MDC.


# Create a bug in bugzilla describing what your patch will do/fix, or use an existing bug if appropriate
# Create a bug in bugzilla describing what your patch will do/fix, or use an existing bug if appropriate
# If the patch fixes a bug or adds a feature (rather than, say, code clean-up, string changes, style changes, etc), it should have a unit test.   
# If the patch fixes a bug or adds a feature (rather than, say, code clean-up, string changes, style changes, etc), it should have a unit test.   
# Regardless of whether you're adding a test, try all the browser chrome tests locally: '''make -C ''<objdir>'' mochitest-browser-chrome'''
# Regardless of whether you're adding a test, try all the browser chrome tests locally: '''make -C ''<objdir>'' mochitest-browser-chrome'''
# Attach your patch to the bug and assign it for feedback from one of the Panorama team  
# Attach your patch to the bug and assign it for feedback from one of the Panorama team (mitcho, raymondlee, seandunn)
# Once you've got f+, mark it r? for review (dietrich, dolske, gavin are good candidates; ask in #tabcandy if you're not sure who to assign it to)
# Once you've got f+, mark it r? for review (generally to iangilman, but other potentials include dao, dietrich, dolske, gavin; ask in #tabcandy if you're not sure who to assign it to)
# Once they've given you an r+, you'll need approval (unless it's already marked as a blocker, which means it's already approved for landing):
## Mark it with a question mark under "approval 2.0", if the reviewer hasn't already done so
## If it's a rush, poke someone to do the approval 2.0 (beltzner, johnath, dietrich, gavin, dolske are good candidates, or ask in #tabcandy if unsure who)
# Push your patch to tryserver to make sure it doesn't break anything:  
# Push your patch to tryserver to make sure it doesn't break anything:  
## Update your m-c local repo  
## Update your m-c local repo  
Line 120: Line 136:
## hg push -f ssh://hg.mozilla.org/try
## hg push -f ssh://hg.mozilla.org/try
## Check the results at: http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry
## Check the results at: http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry
# Once you've got your approval, make a fresh patch with the commit message giving the bug number and title (if you haven't already done that), and updated with [r=foo a=bar]
## Add a comment to the bug in question with a link to your try run like so: http://tbpl.mozilla.org/?tree=MozillaTry&rev=9a8979f8ef4b
# Once they've given you an r+, you'll need approval (unless it's already marked as a blocker, which means it's already approved for landing, or if it's a test fix):
## Mark it with a question mark under "approval 2.0", if the reviewer hasn't already done so
## If it's a rush, poke someone to do the approval 2.0 (beltzner, johnath, dietrich, gavin, dolske are good candidates, or ask in #tabcandy if unsure who)
# Once you've got your approval, make a fresh patch with the commit message giving the bug number and title (if you haven't already done that), and updated with "r=foo a=bar"
# Mark the bug for checkin, by adding the keyword "checkin-needed" to it.
# Mark the bug for checkin, by adding the keyword "checkin-needed" to it.
# Add it as a "ride-along" on the landing queue: https://wiki.mozilla.org/LandingQueue
# If it's urgent, poke someone who can land it, such as iangilman or ehsan
# If it's urgent, poke someone who can land it


Also, make sure to read http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Also, make sure to read http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
= Reading the tinderbox tea leaves =
Once you've pushed to a repo that builds (such as mozilla-central or try), go to the tinderbox pushlog (here are the links for [http://tests.themasta.com/tinderboxpushlog/ mozilla-central] and [http://tests.themasta.com/tinderboxpushlog/?tree=MozillaTry try]). Shortly your push should show up. Watch the letters to the right of your push for signs of danger.
* Gray means it hasn't run yet
* Green means it's good
* Red is for broken builds
* Orange is for tests that fail
When you get an orange, you need to determine if it's your fault, or if it's a known intermittent failure. The first thing to do is click on the orange letter; it'll bring up a bank of info at the bottom of the window, along with a blue panel listing potential bug matches. If you see an open bug that looks like a likely match, you're good. If this is on mozilla-central, hit the "add a comment" link in the lower-left to star it. Note that if the matching bug is not suggested, you can just type in the word "bug" and the bug number in the "comment" dialog, but please then also add the url to the log into the bug.
If this looks like a new issue, try the "view brief log" or "view full log" links in the lower left for clues. The failure should be listed at the top, and additional useful information may be found at the very bottom prefixed by "TinderboxPrint:". If you still don't think it's your bug, try searching bugzilla for a match. If no luck, try asking around on IRC.


= Merging mozilla-central to tabcandy-central =  
= Merging mozilla-central to tabcandy-central =  
Line 138: Line 170:


Also, don't forget to rebuild minefield with all the new code to make sure nothing's broken. Whether you do this before or after pushing is up to your sense of risk. :)
Also, don't forget to rebuild minefield with all the new code to make sure nothing's broken. Whether you do this before or after pushing is up to your sense of risk. :)
= Committing to mozilla-central =
Also see [https://developer.mozilla.org/en/Developer_Guide/Committing_Rules_and_Responsibilities Committing Rules and Responsibilities] on MDC as well as [[Tree Rules]].
You'll need [http://www.mozilla.org/hacking/commit-access-policy/ level 3 access] and a patch that's ready to go (see the section on Landing, above). These instructions assume you're using Mercurial Queues.
# Take a [http://tests.themasta.com/tinderboxpushlog/ look at the tree] to make sure the coast is clear
# Check in with the sheriff on duty (you can find them on the user list for #developers on IRC)
# In bugzilla, grab the URL patch you want to apply (e.g. https://bug601863.bugzilla.mozilla.org/attachment.cgi?id=480853)
# In the terminal, go to your local repository and make sure you don't have any unpushed changes
# '''hg qimport ''<patch url>'' && hg qpush'''
# '''hg pull -u --rebase'''
# Take care of merge conflicts if needed
# Make sure the patch has the correct commit message including bug number, bug title, and reviewers. '''hg qrefresh -e'''
# Make sure the patch has the correct username. '''hg qrefresh -u 'User Name <user@domain.com>''''
# '''hg qfinish -a'''
# '''hg outgoing''' to make sure everything looks cool
# '''hg push'''
# Update the bug with a link to the change (e.g. https://hg.mozilla.org/mozilla-central/rev/beb044c602a7), and resolve it as fixed.
# [https://developer.mozilla.org/en/Developer_Guide/Committing_Rules_and_Responsibilities#Watch_The_Tree Watch the tree] to make sure you didn't burn it down!


= Code Documentation =
= Code Documentation =
Line 146: Line 199:


Even though the TabCandy code is now in the tabcandy-central branch, we're still keeping the documentation in the old tabcandy branch. The docs script, [http://hg.mozilla.org/labs/tabcandy/file/tip/tcc-docs.sh tcc-docs.sh], assumes your tabcandy-central folder is next to your tabcandy folder.
Even though the TabCandy code is now in the tabcandy-central branch, we're still keeping the documentation in the old tabcandy branch. The docs script, [http://hg.mozilla.org/labs/tabcandy/file/tip/tcc-docs.sh tcc-docs.sh], assumes your tabcandy-central folder is next to your tabcandy folder.
= Z-Index Map =
* -999999: background gradient
* -103 to -101: trenches
* -99 to -1: Phantom groups (from dragging on the background, or stacking tabs on each other)
* 0 to 99996: groups and tabs. Each time a group or orphan tab is dragged, its z-index is incremented 1. All of a group's children tabs are indexed in order above the group.
* 99997 to 99999: expanded stack shield, tray, tabs
* 999999: "front" for zoom and drag
* 1000000: Action buttons (search, exit)
* 1000001: Search shade
* 1000010: search result tabs
* 1000050: Search entry box


= Style Guide =  
= Style Guide =  


Use spaces, not tabs, with two spaces per "tab stop".  
* Use spaces, not tabs, with two spaces per "tab stop".
* 80 character line length limit.
* Prefer native methods over libraries where appropriate. For instance, use Array.forEach() rather than iQ.each() (note that for each(...in...) [https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Statements/for...in#Description has issues] and should generally be avoided).
* Assert text should be written in terms of what should happen rather than what did or did not happen, e.g. Utils.assert(a == 2, "a should be equal to 2"). This way it's clear both when reading the code and when reading the console/log.
 
= Do No Harm =
 
In terms of breakage, we should rank them in the following scale, based on the principle of "do no harm":
 
# Anything that breaks the experience for people who never enter Panorama mode (i.e., start with a fresh profile and use the browser normally, never once entering the Panorama UI (or even triggering Panorama, such as happens when you pull up the right-click menu on a tab in the tab strip)). We should be good here, since in this scenario Panorama just lays dormant, but of course we need to verify.
# Anything that breaks if you enter Panorama but then exit immediately and otherwise continue to use the browser normally. In this scenario, the browser should behave as it always has, even though Panorama is in the background watching things. Again for testing this should start with a fresh profile (or at least kill your sessionstore). Also note that you need to initialize Panorama once per session.
# Anything that breaks if you use Panorama extensively, but with only one window open in your browser.
# Anything that breaks if you use Panorama with multiple windows open.
 
Sub-scenarios in the bottom three categories include starting Panorama right at the beginning of your session versus using the browser for a while and then starting Panorama.


80 character line length limit.
Within this hierarchy of course we have the usual prioritization from data loss at the top, down to annoyance at the bottom.


Prefer native methods over libraries where appropriate. For instance, use Array.forEach() rather than iQ.each() (note that for each(...in...) [https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Statements/for...in#Description has issues] and should generally be avoided).
Please use this hierarchy to guide your testing and bug fixing.
68

edits