Gavin's blog

Last week fx-team got together in Toronto to talk about what work we’ve done, the work we have planned for the rest of the year, and to kick start some work on Kilimanjaro-related bugs. We had a round of lightning talks on Wednesday, and I gave one entitled “you should review code”. It’s a little bit audience specific – the call to action at the end of the talk is targeted to core Firefox contributors, but the earlier points about effective code review is relevant to anyone who reviews code, even outside of Mozilla. I posted my slides from the talk, as well as a video (WebM, also available on vid.ly).

I’ve made some additions to the list of Firefox reviewers:

• Neil Deakin, a long-time Mozilla contributor who knows XUL inside and out, and has plenty of experience writing and reviewing Mozilla code
• Blair McBride, owner of the add-ons manager, and an existing toolkit reviewer
• Felipe Gomes, a jack-of-all-trades who knows browser code well from his work on touch UI, e10s, and other platform integration bits

These guys have been living by and helping promote the Firefox review guidelines for a long time already, so it’s about time that I get around to recognizing that officially. You should ask them for review next time you’re patching Firefox code!

• “I disagree with the idea promoted by the post, and it upset me, made me feel very uncomfortable and unwelcome”
• “The post encouraged people to support a law that prevents gay people from using the word ‘marriage’”

• “The post is hate speech”
• “The post denies the humanity of a marginalized minority group”

For some people, these statements are roughly equivalent. For others, the distinction is quite obvious. I think the degree to which the post offends you affects your ability to make the distinction. I don’t think any of these perspectives is inherently more legitimate than the other – it’s just a matter of perception.

I think the distinction isn’t very important, particularly when compared to the larger issue of a post making a significant group of people feel uncomfortable. But that doesn’t change the fact that a good number of people (particularly those who were not as offended by the post) will be bothered by the conflation, will perceive the second set of statements to be hyperbole, and will be compelled to point them out as such as part of a debate. I think the people who do this do it not because they want to marginalize the other valid concerns about the post itself, but because to them, the hyperbole is just as (or nearly as) offensive as those other concerns. Again a matter of perception.

I think that it’s important, when trying to make a point, to be thoughtful about which variants of these kinds of statements you use. I think both sets of statements identify problems that most reasonable people would agree need to be addressed (and might even agree about how to address them!). But the set that can be perceived as hyperbole is less effective at creating empathy and shared understanding, particularly in a very tech-y community like ours, because the distraction of semantics leads the debate into unproductive (and secondary) areas.

Shaver posted about this in dev.apps.firefox last week, but I thought it might bear repeating to the Planet audience: as of last week, the Firefox module has a new module owner (yours truly), and a new review policy.

The new review policy is explained in detail on the Wiki. As part of this change, there is now a larger set of people deemed “Firefox reviewers“, and a new policy for nominating reviewers. Together these changes will hopefully address the review bandwidth issues we’ve been having.

Because the Firefox module is relatively large and covers many different code areas, Firefox reviewers are expected to know their areas of expertise, and not be shy about redirecting requests to a more suitable reviewer, if applicable. The wiki page also goes on in detail about what r+ means in the Firefox module – a handy list to review for new and old reviewers alike.

Please, send me email if anything on that page isn’t clear, or if you think anything (or anyone!) is missing!

I don’t recall precisely how I first got involved with Mozilla, but a large part of my early contributions was triaging bugs. I stumbled across Bugzilla at some point, and I really enjoyed finding duplicates and trying to figure out the root causes of bugs. I would watch the “bugs filed today” query, and try to triage as many bugs as I could. It was a fun challenge to find duplicates quickly, and memorize common issues (this, and later IRC- and bonsai-watching habits and general Mozilla addiction are what led to the coining of the name “gavinbot“). Later, as I took on more responsibility in other areas (e.g. development and code review), I stopped being as actively involved in Bugzilla triage itself; the bugmail hasn’t stopped, though

Recently Tyler posted about his frustrations with the bug triage effort. He raises some good points, but I think one of his premises is faulty:  I don’t think that focusing on the number of UNCO bugs is useful. We should focus on outcomes, not on driving a number to 0 for the sake of it.

Are we missing bugs that we should have noticed, but failed to? Maybe, but we’re never going to be 100% on top of every bug, and I don’t think the situation here is as dire as “5000 untriaged bugs!” makes it sound – a very large percentage of those are duplicates, or invalid, or minor issues that we shouldn’t be focusing on. Spending contributor time sorting through many of those bugs probably doesn’t have a very good cost/benefit ratio. Important issues still tend to bubble up pretty effectively, in my experience.

Are people being discouraged from reporting problems to us because they get ignored? This is a problem, certainly (though again, I’m not sure about the degree). We should fix this by having better tools for giving feedback (i.e. not Bugzilla), and there are already successful efforts underway to do that (e.g. input.mozilla.com).

It’s important to step back and look at the bigger picture – driving a Bugzilla query of untriaged bugs to 0 should only be a goal insofar as it helps us achieve the primary goal of producing good software. I contend that it probably isn’t the most effective way to contribute to that goal at the moment.

Getting Firefox to support multiple processes is not a small task. To satisfy the goals for going to a multi-process model, we’re going to be making changes to how the front-end Firefox application code interacts with Web page content, since all of those interactions will need to occur via cross-process “messages”.

Felipe has done some great work getting basic e10s browser functionality landed on mozilla-central. He’s added an –enable-e10s-compat build-time switch that both sets the pref that enables putting tabs into separate processes, and then also disables a bunch of code that doesn’t work in that situation. The disabled code includes some pretty major things like “updating the security/location bar UI in response to page navigation”. He’s working on getting those pieces working in bug 666801, and the rest of the currently known work is being tracked in bug fxe10s.

As we continue to identify pieces of code that need Electrolysis-related changes (using both dynamic analysis and static analysis – progress is being made on both of those fronts!), we’ll be adding them to that tracking bug, and poking people to get involved in fixing them. Now that –enable-e10s-compat has landed on the trunk, anyone should feel free to pick some of those bugs up, and land these fixes on mozilla-central (via fx-team, if desired!). Tim Taubert has a great post about the basics of rewriting code to support e10s, and Firefox reviewers should all be getting into the habit of reviewing e10s fixes (and ensuring that newly added code is e10s friendly!).

Like many other Mozillians, I use irssi running in screen to connect to IRC. There are a bunch of advantages (access from anywhere, text-based interface, persistent connection to IRC, geek cred, etc.) as well as some disadvantages (text-based interface, persistent connection to IRC, geek cred, etc.). Before I switched, I thought the text-based interface would be a deal breaker, but I pretty quickly learned to adjust to that. I like it.

I put together a basic irssi-howto for people interested in learning to use irssi for IRC. It’s most useful to people who can use people.mozilla.org (Mozilla Corporation employees and contractors) and want to connect to irc.mozilla.org, but it could be relevant to anyone who wants to run irssi(+screen) on their own. The scripts are a mix of scripts I’ve written and scripts I’ve found. They do useful things like remembering channels, remembering channel passwords, and syncing screen and IRC away state.

On Monday I finally landed the new “PopupNotifications” JS module, used to display popup-style notifications (as discussed earlier). Since then I’ve spent some time fixing followups, but there’s still a bunch of work to be done (including fleshing out the MDC documentation and trying to find help making menu-buttons nicer). Dave‘s already got a patch up to use the new notifications for the Addons Manager, and Robert is working on getting them working in SeaMonkey.

I completed some reviews, including some for relatively exciting patches from Dão:

• Turning on and fixing the position of the “Firefox” menu button on Windows Vista/7
• Making tabs-on-top the default on Windows (see Alex’s post for more details)

I also spent a bit of time reviewing fixes for dolske‘s prompting code rewrite that landed last week as well as cleaning up the new HUD panel‘s styling.

Next week, I plan on trying to wrap up as much of the notification-related stuff as possible, and move on to getting some patches up for the Account Manager work. Also need to do a bit of prep for the Summit which is coming up quickly!

I’ve made some progress since my last post:

Still rather ugly, but at least it’s functional!

Some things I’ll be looking into next:

• the dismissal behavior – currently dismissing the notification (i.e. clicking elsewhere on the screen) denies the geolocation request. I think we want to instead keep the request pending and allow you to access it later from the site-identity menu, but that requires some extra work (tracking shown/unshown notifications, etc.)
• make notifications triggered from background tabs/windows behave correctly (right now they’re just ignored)
• multiple-notification stacking
• support for different priorities (not sure this will be needed)
• styling! – Stephen is working on mockups for menu-buttons, which is probably the most obvious issue

The current working API is very similar to the existing notificationbox API. It’s definitely not set in stone. Right now it looks like this:

var mainAction = {
  label: "Share Location",
  accessKey: "S",
  callback: function() {
    request.allow();
  },
};

var secondaryActions = [
  {
    label: "Always Share",
    accessKey: "A",
    callback: function () {
      Services.perms.add(requestingURI, "geo", ALLOW_ACTION);
      request.allow();
    }
  },
  {
    label: "Never Share",
    accessKey: "N",
    callback: function () {
      Services.perms.add(requestingURI, "geo", DENY_ACTION);
      request.cancel();
    }
  }
];

var dismissalAction = {
  callback: function() {
    request.cancel();
  }
};

Notification.show("geolocation", "title", message,
                  "chrome://browser/skin/Geo.png",
                  mainAction, secondaryActions,
                  dismissalAction, browser);

Just recently I started working on implementing a new notification system for Firefox. It’s been discussed and blogged about before, and both Neil and MattN have worked on it before, so the task mostly involves just updating, cleaning up and completing their work. It’s being tracked by bug 398776, and I’m still in the early stages. Here’s what I threw together just so that I could test the API:

That still needs some CSS tweaking, obviously (in general, and for the button specifically: bug 509642), and Neil’s work in bug 554937 should make showing the actual popup even easier.

I also did a few other things of note this week:

• worked on some search service changes to enable changes to the Google search plugin that should make the search experience better
• did a bit of experimenting with moving some browser.js code to a JS module – nothing real yet
• worked on a security bug
• did some code reviews