Bug 68045 – Generate WebKit team's page out of committers.py

• Component: WebKit Website
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Given how little attention we give to http://trac.webkit.org/wiki/WebKit%20Team, it's probably better if we merged that information into http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py which is updated more frequently. Also, full name, nick, etc... are duplicated on those two pages.

[Ryosuke Niwa]

Created attachment 107269 [details]
prototype (kind of works for reviewers)

[Ryosuke Niwa]

Created attachment 107270 [details]
screenshot of the prototype

[Ryosuke Niwa]

Created attachment 107296 [details]
prototype 2

This version can parse http://trac.webkit.org/wiki/WebKit%20Team and add annotation on the page.
Open fetch-contributors.html?annotate to turn on this feature.

[Ryosuke Niwa]

Created attachment 107300 [details]
prototype 3

Because filling up affiliation is quite tedious task, I'm experimenting with automatically inferring affiliation from their email addresses. e.g. if you have @google.com address, then you're affiliated Google and so forth.

The problem is that a big chunk of Googlers are using @chromium.org address yet I can't assume everyone who uses @chromium.org addresses are Googlers.

So there are people like leviw who used to work at Apple but currently working for Google. anttik has both Nokia and Apple addresses as well.  So I'm not sure what the good approach is.

[Adam Barth]

Maybe just list the domains from the email address as the affiliation?  So the Chromium folks would be listed as Chromium?  Another approach is to just drop the affiliation.  It's not the most important quality anyway.

[Ryosuke Niwa]

(In reply to comment #5)
> Maybe just list the domains from the email address as the affiliation?  So the Chromium folks would be listed as Chromium?

Right, that's what I'm doing now. But in the case of leviw / anttik, they end up having affiliations with multiple corporations, which they may or may not like.  So I think we should let contributors override it in committers.py.

> Another approach is to just drop the affiliation.  It's not the most important quality anyway.

Yeah, that's a viable option for me as well but some people might find it useful so I'd like to keep them.

Here are some changes we have to make to committers.py
- multiple nick names should be supported
- port information (e.g. paroga is a WinCE maintainer)
- affiliation override

I don't want to let people manually add area of expertise and other random comments in committers.py for now as I'm trying to somehow auto-generate it from change logs (see bug 68061).

[Eric Seidel (no email)]

I worry you may be a bit covered in yak hair at this point.

[Adam Barth]

How dare you impune rniwa's choice of hair!  He and I are both on the blue team.

[Evan Martin]

Back when I played with committer data I found that mapping emails to affiliations works for the easy cases (apple, etc) but breaks down in the long tail.

See "domain_companies" and "other" (lines 50-80) in
https://github.com/martine/webkit-who/blob/master/webkit.py

[Ryosuke Niwa]

(In reply to comment #9)
> Back when I played with committer data I found that mapping emails to affiliations works for the easy cases (apple, etc) but breaks down in the long tail.

Yeah, we can't even assume all contributors with @chromium.org are Googlers either. (honten is a good counterexample).

> See "domain_companies" and "other" (lines 50-80) in
> https://github.com/martine/webkit-who/blob/master/webkit.py

Very nice.

[Alexis Menard (darktears)]

(In reply to comment #9)
> Back when I played with committer data I found that mapping emails to affiliations works for the easy cases (apple, etc) but breaks down in the long tail.

Yep look as us @openbossa.org . We are not really Nokia, but we work on the Qt port and our official company name is "Instituto Nokia de Tecnologia" -> INdT. openbossa is our open source face :D.

> 
> See "domain_companies" and "other" (lines 50-80) in
> https://github.com/martine/webkit-who/blob/master/webkit.py

[Eric Seidel (no email)]

Evan's (unintentional?) point that using a separate datastore for the affiliation information makes a lot of sense!

We could add a separate affiliations/teams/whatever.py file next to comitters.py which contains mappings from emails to teams separately from whether people are committers/contributors, etc.

[Ryosuke Niwa]

Created attachment 107404 [details]
initial version

[Ryosuke Niwa]

cc js wiz in the hope they can improve my js.

[Ryosuke Niwa]

Comment on attachment 107404 [details]
initial version

View in context: https://bugs.webkit.org/attachment.cgi?id=107404&action=review

> Websites/webkit.org/team.html:137
> +        function parseWiki(text) {

This function exists for debugging purposes.  It's used with ?annotate to compare information on committers.py and wiki page.

> Websites/webkit.org/team.html:182
> +        function annotateWithWikiData() {

Ditto.

[Ryosuke Niwa]

Created attachment 107406 [details]
screenshot

[Ryosuke Niwa]

Comment on attachment 107404 [details]
initial version

View in context: https://bugs.webkit.org/attachment.cgi?id=107404&action=review

> Websites/webkit.org/team.html:59
> +                        var nameEmailsNicks = eval('[' + match[2].replace(/^u"/,'"').replace('None, ', 'null, ') + ']');

I'm not sure if I should sanity check the value in match[2] in the case someone injects malicious code in committers.py.
It seems very unlikely but...

[Eric Seidel (no email)]

I am not your man for this review.  It's lame that we have to manually add the header ifnormation.  Are you sure we can't make this use a php include like the other pages do?

[Ryosuke Niwa]

(In reply to comment #18)
> I am not your man for this review.  It's lame that we have to manually add the header ifnormation.  Are you sure we can't make this use a php include like the other pages do?

Oops! I didn't realize that we did this. Of course we should be able to php-include them. Will fix.

[Ryosuke Niwa]

Created attachment 107426 [details]
Fixed per Eric's comment

[Ryosuke Niwa]

Created attachment 107428 [details]
Added more entries to domainAffiliations

[Leandro Pereira]

View in context: https://bugs.webkit.org/attachment.cgi?id=107426&action=review

> Websites/webkit.org/team.html:28
> +

Would be nice if a 'profusion.mobi': 'ProFUSION' line were added here :)

> Websites/webkit.org/team.html:31
> +    'kde.org': 'KDE',

Please remove the comma at the end of this line. (In JavaScript collections, commas are used only as separators.)

> Websites/webkit.org/team.html:50
> +                area: nameEmailsNicks[3],

Ditto.

> Websites/webkit.org/team.html:102
> +    name = name.toLowerCase();

This variable doesn't seem to be used anywhere else.

> Websites/webkit.org/team.html:171
> +    window.webKitRetrievedAtLeastOnce = 1;

What's this one used for?

> Websites/webkit.org/team.html:181
> +xhr.onerror = function () { alert('Could not obtain committers.py'); };

I'd set some container's innerHTML to display the error instead of popping up an alert box.

[Ryosuke Niwa]

Created attachment 107439 [details]
Updated per Leandro's comment

[Ryosuke Niwa]

(In reply to comment #22)
> View in context: https://bugs.webkit.org/attachment.cgi?id=107426&action=review
> 
> > Websites/webkit.org/team.html:28
> > +
> 
> Would be nice if a 'profusion.mobi': 'ProFUSION' line were added here :)

Done.

> > Websites/webkit.org/team.html:31
> > +    'kde.org': 'KDE',
> 
> Please remove the comma at the end of this line. (In JavaScript collections, commas are used only as separators.)

Done.

> > Websites/webkit.org/team.html:50
> > +                area: nameEmailsNicks[3],
> 
> Ditto.

Done.

> > Websites/webkit.org/team.html:102
> > +    name = name.toLowerCase();
> 
> This variable doesn't seem to be used anywhere else.
>
> > Websites/webkit.org/team.html:171
> > +    window.webKitRetrievedAtLeastOnce = 1;
> 
> What's this one used for?

Oops, removed those lines.
 
> > Websites/webkit.org/team.html:181
> > +xhr.onerror = function () { alert('Could not obtain committers.py'); };
> 
> I'd set some container's innerHTML to display the error instead of popping up an alert box.

Done.

[Daniel Bates]

Comment on attachment 107439 [details]
Updated per Leandro's comment

View in context: https://bugs.webkit.org/attachment.cgi?id=107439&action=review

This patch has at least one cross-site scripting vulnerability. In particular, when the page is passed the search string "?annotate" the script includes untrusted content from <http://trac.webkit.org>. See my remarks for annotateForContributor() for more details.

> Websites/webkit.org/team.html:47
> +        var contributorLine = /^\s+(Reviewer|Committer|Contributor)\((.+)\),?$/;

It seems sufficient to define this regular expression once instead of on each iteration of this loop. I suggest hoisting this declaration outside of this for-loop.

> Websites/webkit.org/team.html:50
> +                var nameEmailsNicks = eval('[' + match[2].replace(/^u"/,'"').replace('None, ', 'null, ') + ']');

I suggest adding a comment above this line to explain that we massage the Python code extracted from committer.py into a JavaScript-compatible form so as to utilize the JavaScript parser to actually parse the committer.py data and build a data structure to facilitate extracting the Committer/Contributor/Reviewer details.

On another note, the usage of eval() here is OK. Clearly the usage of eval() here assumes that people with Subversion credentials aren't malicious and that such credentials won't be stolen. That being said, if an attacker controls such credentials then they control the repository, including the web site.

> Websites/webkit.org/team.html:150
> +    function annotateForContributor(contributor) {
> +        var listItem = findListChildForContributor(contributor);
> +        if (!listItem) {
> +            var listElement = document.getElementById(contributor.kind + 's');
> +            var listItem = document.createElement('li');
> +            listElement.appendChild(listItem);
> +            listItem.innerHTML = '<span style="background-color:red;">' + createMarkupForContributor(contributor) + '</span>';
> +        } else {
> +            var nicksInContainer = nicksInListItem(listItem);
> +            if (contributor.nicks) {
> +                if (nicksInContainer)
> +                    nicksDiff = contributor.nicks.filter(function (nick) { return nicksInContainer.indexOf(nick) < 0; })
> +                if (!nicksInContainer || nicksDiff.length)
> +                    listItem.innerHTML += ' <span style="background-color:red;">(' + nicksDiff + ')</span>';
> +            }
> +
> +            var affiliationContainer = listItem.getElementsByClassName('affiliation')[0];
> +            var affiliation = formatAffiliation(contributor);
> +            if (affiliation && (!affiliationContainer || affiliationContainer.textContent != affiliation))
> +                listItem.innerHTML += ' <span style="background-color:red;"><em>' + affiliation + '</em></span>';
> +        }
> +    }
> +

Content from <http://trac.webkit.org> should be considered untrusted since anyone can register for a trac account at <https://trac.webkit.org/auth/register/> and edit any wiki page. In particular, an attacker can edit the wiki page <http://trac.webkit.org/wiki/WebKit%20Team>.

Notice that the value of innerHTML is parsed for markup (*) and that annotateForContributor() modifies the DOM via innerHTML with content from <http://trac.webkit.org/wiki/WebKit%20Team> without escaping it.

Without loss of generality, suppose the following markup was appended to the Reviewers section of <http://trac.webkit.org/wiki/WebKit%20Team>:

 * '''<a href='#' onmouseover='alert(1)'>Bob</a>''' (bob) ''ACompany''

Then we visit <http://www.webkit.org/team.html?annotate>. Notice that annotateForContributor() will ultimately add a Reviewer entry for Bob. Hovering over the name Bob will display a JavaScript alert with the message "1" because the HTML markup embedded in the Trac formatting syntax is written verbatim to the page.

> Websites/webkit.org/team.html:153
> +        if (this.status != 200)

Can we strengthen the inequality to "!=="?

> Websites/webkit.org/team.html:177
> +    xhr.onerror = function () { alert('Could not obtain http://trac.webkit.org/wiki/WebKit%20Team'); };
> +    xhr.open('GET', 'http://trac.webkit.org/wiki/WebKit%20Team?format=txt');

The URL "http://trac.webkit.org/wiki/WebKit%20Team" is referenced twice. I suggest defining a variable, say webkitTeamWikiURL, for this value so that we can reference this URL in both XMLHttpRequest.onerror handler and XMLHttpRequest.open(). We may also want to consider making this variable global and having it at the top of this script to both increase its visibility and facilitate changes.

> Websites/webkit.org/team.html:183
> +    if (this.status != 200)

Can we strengthen the inequality to "!=="?

> Websites/webkit.org/team.html:195
> +xhr.open('GET', 'http://svn.webkit.org/repository/webkit/trunk/Tools/Scripts/webkitpy/common/config/committers.py');

For your consideration, I suggest defining a global variable for the SVN URL and putting this variable at the top of this script as a means to centralize this configuration data and facilitate changes.

[Daniel Bates]

I forgot to include the footnote (*) in my last comment:

(*) As per section 3.5.5 of the HTML5 spec. <http://www.w3.org/TR/html5/apis-in-html-documents.html#innerhtml>.

[Ryosuke Niwa]

(In reply to comment #25)
> Content from <http://trac.webkit.org> should be considered untrusted since anyone can register for a trac account at <https://trac.webkit.org/auth/register/> and edit any wiki page. In particular, an attacker can edit the wiki page <http://trac.webkit.org/wiki/WebKit%20Team>.

Oops! That's a good point. Fixed.

[Ryosuke Niwa]

Created attachment 107459 [details]
Fixed XSS issue

[Ryosuke Niwa]

Comment on attachment 107459 [details]
Fixed XSS issue

View in context: https://bugs.webkit.org/attachment.cgi?id=107459&action=review

> Websites/webkit.org/team.html:154
> +            var affiliationContainer = listItem.getElementsByClassName('affiliation')[0];
> +            var affiliation = formatAffiliation(contributor);

Note I've removed annotation for nicknames because committers.py has been updated to include all nicknames listed on the wiki page.

[Patrick R. Gansterer]

Did anyone thought about implementing this in php on the serverside (for JS disabled browsers)?

[Ryosuke Niwa]

(In reply to comment #30)
> Did anyone thought about implementing this in php on the serverside (for JS disabled browsers)?

I have but I'll probably do that only after the major features are implemented since it's much easier to do everything in javascript.

[Erik Arvidsson]

Comment on attachment 107459 [details]
Fixed XSS issue

View in context: https://bugs.webkit.org/attachment.cgi?id=107459&action=review

> Websites/webkit.org/team.html:51
> +                var nameEmailsNicks = eval('[' + match[2].replace(/^u"/,'"').replace('None, ', 'null, ') + ']');

Don't use eval. Maybe use JSON.parse if it can be fixed up to be JSON. Maybe if I knew what the data looks like it would be more clear.

> Websites/webkit.org/team.html:75
> +    for (domain in domainAffiliations) {

missing var

> Websites/webkit.org/team.html:102
> +    affiliation = formatAffiliation(contributor);

missing var. It might be good to use strict to find these for you.

> Websites/webkit.org/team.html:120
> +    var nicksContainer = listItem.getElementsByClassName('nicks')[0];

You can use querySelector here which is cleaner, especially if you only care about one element.

listItem.querySelector('.nicks')

> Websites/webkit.org/team.html:153
> +            var affiliationContainer = listItem.getElementsByClassName('affiliation')[0];

querySelector

> Websites/webkit.org/team.html:169
> +        var currentKind = null;

There is usually no need to assign null to vars. Variables start of as undefined which is usually just as good.

> Websites/webkit.org/team.html:177
> +            match = lines[i].replace(/[\{\}<>"%;\&+\/]/g, '').match(/^\s+\*\s+\'\'\'([^']+)\'\'\'\s*(\(([^']+)\)\s*)?(\'\'([^']+)\'\')?\s*$/);

Sorry, this is impossible to read. I know we have a policy of no comments but can this either be done differently or the rule of no comment needs to be broken here.

[David Levin]

(In reply to comment #32)
> (From update of attachment 107459 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107459&action=review
> > Websites/webkit.org/team.html:177
> > +            match = lines[i].replace(/[\{\}<>"%;\&+\/]/g, '').match(/^\s+\*\s+\'\'\'([^']+)\'\'\'\s*(\(([^']+)\)\s*)?(\'\'([^']+)\'\')?\s*$/);
> 
> Sorry, this is impossible to read. I know we have a policy of no comments but can this either be done differently or the rule of no comment needs to be broken here.

Who/What has a policy of no comments?

[Ryosuke Niwa]

Created attachment 107509 [details]
Used JSON instead of eval and querySelector instead of getElementsByClassName

[Daniel Bates]

Comment on attachment 107509 [details]
Used JSON instead of eval and querySelector instead of getElementsByClassName

View in context: https://bugs.webkit.org/attachment.cgi?id=107509&action=review

This patch doesn't add a hyperlink to this page and hence you can only access it directly by its name.  I take it that you plan to work on this some more before adding a hyperlink to this page from the public website/wiki?

Additionally, as Patrick Gansterer implied by his question in comment 30, it would be great if we provided some kind of fall back when JavaScript is either disabled or the client lacks sufficient JavaScript support. We may not be interested in explicitly supporting the latter group.

> Websites/webkit.org/team.html:30
> +    'rim.com': 'Research in Motion',

Nit: Research in Motion => Research In Motion. That being said, we should fix this on the wiki as well.

> Websites/webkit.org/team.html:49
> +        if (match = contributorLine.exec(lines[i])) {

It seems sufficient to define variable match as a local variable instead of a global. That is, I would write this line as:

var match = contributorLine.exec(lines[i]);
if (match) {
...

You may also want to consider using an early-continue style. This may make the code flow a bit easier to follow given that there is a "continue" statement in the "catch" clause of the try-block. Using an early-continue style would also remove one level of indentation and make the two continue statements left-align.

> Websites/webkit.org/team.html:81
> +    return affiliations ? affiliations.join(' / ') : null;

Notice that affiliations.join(' / ') == "" (the empty string) when affiliations == [] and the empty string evaluates to false in a boolean context.

Therefore, I would write this line as:

return affiliations.join(' / ');

> Websites/webkit.org/team.html:94
> +function pupulateContributorListItem(listItem, contributor) {

pupulateContributorListItem => populateContributorListItem

(Notice the 'o' in populate)

> Websites/webkit.org/team.html:115
> +        pupulateContributorListItem(listItem, contributorsOfKind[i]);

Ditto.

> Websites/webkit.org/team.html:130
> +        if (nameContainer && nameContainer.textContent.toLowerCase().indexOf(contributor.name.toLowerCase()) >= 0)

The expression "contributor.name.toLowerCase()" evaluates to the same result for each iteration of this loop. I suggest storing the result of this expression in a variable, say contributorName or lowercaseContributorName, outside of the for-loop instead of re-evaluting this expression on each iteration.

> Websites/webkit.org/team.html:133
> +        var nicksInContainer = nicksInListItem(listChildren[i]);
> +        if (nicksInContainer && contributor.nicks) {

It's unnecessary to call nicksInListItem() if contributor.nicks is null. That being said, I take it you ordered it this way since the majority of people list a nick on the Trac wiki page; => less likely contributor.nicks is null; => less likely that this if-condition will evaluate to false.

If this was your intention then, for your consideration, you may want to add a comment here that explains this assumption.

> Websites/webkit.org/team.html:151
> +            pupulateContributorListItem(listItem, contributor);

pupulateContributorListItem => populateContributorListItem

(Notice the 'o' in populate)

> Websites/webkit.org/team.html:176
> +            // Strip special html characters, and match lines like * '''Ryosuke Niwa''' (rniwa) ''Google''

html => HTML

(because it's an acronym)

> Websites/webkit.org/team.html:177
> +            match = lines[i].replace(/[\{\}<>"%;\&+\/]/g, '').match(/^\s+\*\s+\'\'\'([^']+)\'\'\'\s*(\(([^']+)\)\s*)?(\'\'([^']+)\'\')?\s*$/);

From my understanding, the only characters that need to be escaped within a character class expression (i.e. [ ... ]) are '\', ']', and '-' since they represent the escape character, end of the character class, and the class range separator, respectively. That is, it's unnecessary to escape the literals '{', '}', '&', and '/' inside a character class. So, "lines[i].replace(/[\{\}<>"%;\&+\/]/g, '')" can be simplified to:

lines[i].replace(/[{}<>"%;&+/]/g, '').

Also, a ' (single-quote) doesn't need to be escaped within a regular expression. So, the last regular expression on this line can be written as:

/^\s+\*\s+'''([^']+)'''\s*(\(([^']+)\)\s*)?(''([^']+)'')?\s*$/;

You may also want to define a variable for this expression with a more meaningful name outside of this for-loop, maybe webkitTeamWikiNameAndNickAndAffliationRegEx? or webkitTeamWikiNameRegEx? or nameAndNickAndAffliationRegEx?

> Websites/webkit.org/team.html:188
> +    xhr.onerror = function () { alert('Could not obtain http://trac.webkit.org/wiki/WebKit%20Team'); };

How did you come to the decision to use a JavaScript alert() for displaying the error message? Can we write this error message to the page instead of displaying a modal dialog alert()?

Writing the error message to the page would also be consistent with the approach you've taken when the committers.py file cannot be obtained on line 206.

[Ryosuke Niwa]

Thanks for the review!

(In reply to comment #35)
> (From update of attachment 107509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107509&action=review
> 
> This patch doesn't add a hyperlink to this page and hence you can only access it directly by its name.  I take it that you plan to work on this some more before adding a hyperlink to this page from the public website/wiki?

Yes.

> Additionally, as Patrick Gansterer implied by his question in comment 30, it would be great if we provided some kind of fall back when JavaScript is either disabled or the client lacks sufficient JavaScript support. We may not be interested in explicitly supporting the latter group.

I'm intending to convert this into a php script at some point in the future. And that's probably when I add the link on website. I can probably add a link to wiki much earlier though.

> > Websites/webkit.org/team.html:30
> > +    'rim.com': 'Research in Motion',
> 
> Nit: Research in Motion => Research In Motion. That being said, we should fix this on the wiki as well.

Will fix.

> > Websites/webkit.org/team.html:49
> > +        if (match = contributorLine.exec(lines[i])) {
> 
...
> You may also want to consider using an early-continue style. This may make the code flow a bit easier to follow given that there is a "continue" statement in the "catch" clause of the try-block. Using an early-continue style would also remove one level of indentation and make the two continue statements left-align.

Good point. will do continue.

> > Websites/webkit.org/team.html:81
> > +    return affiliations ? affiliations.join(' / ') : null;
> 
> Notice that affiliations.join(' / ') == "" (the empty string) when affiliations == [] and the empty string evaluates to false in a boolean context.

The problem is that affiliations can be null/undefined. I'll keep the original code here.

> > Websites/webkit.org/team.html:94
> > +function pupulateContributorListItem(listItem, contributor) {
> 
> pupulateContributorListItem => populateContributorListItem
> 
> (Notice the 'o' in populate)

Will fix.

> > Websites/webkit.org/team.html:130
> > +        if (nameContainer && nameContainer.textContent.toLowerCase().indexOf(contributor.name.toLowerCase()) >= 0)
> 
> The expression "contributor.name.toLowerCase()" evaluates to the same result for each iteration of this loop. I suggest storing the result of this expression in a variable, say contributorName or lowercaseContributorName, outside of the for-loop instead of re-evaluting this expression on each iteration.

I'll keep it inside the loop since I'm not concerned about the performance at all, and it doesn't necessarily improve the readability.

> > Websites/webkit.org/team.html:133
> > +        var nicksInContainer = nicksInListItem(listChildren[i]);
> > +        if (nicksInContainer && contributor.nicks) {
> 
> It's unnecessary to call nicksInListItem() if contributor.nicks is null. That being said, I take it you ordered it this way since the majority of people list a nick on the Trac wiki page; => less likely contributor.nicks is null; => less likely that this if-condition will evaluate to false.
>
> If this was your intention then, for your consideration, you may want to add a comment here that explains this assumption.

As I noted, I didn't take any performance consideration when I wrote this code. As I'm intending to replace this entire code with php before the release, I'm going to keep the code as is.

> > Websites/webkit.org/team.html:176
> > +            // Strip special html characters, and match lines like * '''Ryosuke Niwa''' (rniwa) ''Google''
> 
> html => HTML

Will fix.

> > Websites/webkit.org/team.html:177
> > +            match = lines[i].replace(/[\{\}<>"%;\&+\/]/g, '').match(/^\s+\*\s+\'\'\'([^']+)\'\'\'\s*(\(([^']+)\)\s*)?(\'\'([^']+)\'\')?\s*$/);
> 
> From my understanding, the only characters that need to be escaped within a character class expression (i.e. [ ... ]) are '\', ']', and '-' since they represent the escape character, end of the character class, and the class range separator, respectively. That is, it's unnecessary to escape the literals '{', '}', '&', and '/' inside a character class. So, "lines[i].replace(/[\{\}<>"%;\&+\/]/g, '')" can be simplified to:
> 
> lines[i].replace(/[{}<>"%;&+/]/g, '').

Will do.

> Also, a ' (single-quote) doesn't need to be escaped within a regular expression. So, the last regular expression on this line can be written as:
> 
> /^\s+\*\s+'''([^']+)'''\s*(\(([^']+)\)\s*)?(''([^']+)'')?\s*$/;

Good point. Will do.

> You may also want to define a variable for this expression with a more meaningful name outside of this for-loop, maybe webkitTeamWikiNameAndNickAndAffliationRegEx? or webkitTeamWikiNameRegEx? or nameAndNickAndAffliationRegEx?

Will use teamWikiContributorEntryPattern

> > Websites/webkit.org/team.html:188
> > +    xhr.onerror = function () { alert('Could not obtain http://trac.webkit.org/wiki/WebKit%20Team'); };
> 
> How did you come to the decision to use a JavaScript alert() for displaying the error message? Can we write this error message to the page instead of displaying a modal dialog alert()?

For committers.py case, I'm replacing the entire page with the error message but doing so for wiki page is problematic because that'll override the list of contributors already loaded on the page.  Since this is a debugging feature, I'll leave it as is.

[Ryosuke Niwa]

Committed r95578: <http://trac.webkit.org/changeset/95578>

[Ryosuke Niwa]

Committed r95579: <http://trac.webkit.org/changeset/95579>