Bug 971110/Bug 1118255 landed code in toolkit which depend on browser. This isn't cool, as it means we can't use this panel for Instantbird/Thunderbird. (And is blocking some debugging for bug 1018060.) Can we remove the dependency on React for this page? I understand there was a push to land this due to deadlines, but it landed in Mozilla 35 and is still there. The exact error is: > Timestamp: 2/4/15, 8:09:21 PM > Error: ReferenceError: React is not defined > Source File: chrome://global/content/aboutwebrtc/aboutWebrtc.js > Line: 11 (Also, the patches in those bugs don't look like they were reviewed by a toolkit peer? Or a Firefox peer, but that wasn't in effect at the time. :) )

I'm not against the library moving as long as loop devs can update it whenever we need to without additional review. For it to be moved, there will be several things to be done: - Find a place for it (I know devtools are interested as well) - Do the normal code moves - Make Loop's standalone server [1] serve the file at the same place(s) as it does now - Ensure all tests still pass - Update loop-client's [1] extract_from_hg.py to extract from the new location into the original location in the loop-client repo [1] http://mxr.mozilla.org/mozilla-central/source/browser/components/loop/standalone/ [2] https://github.com/mozilla/loop-client

We'll need to backout the offending changeset, I'm afraid. Moving React to toolkit is not applicable at this time; when devtools starts using it, we'll most likely move it to be a browser module, not to toolkit. It looks like the updated about:webrtc wasn't in need of React as well, the dynamic content could also be implemented with relatively simple vanilla JS. I don't think the additional work that Mark mentioned in comment 1 makes for a good added value proposition, considering the amount of effort required to tick all the boxes. It's unfortunate that no toolkit peer has reviewed that patch as I'm pretty sure this would've been noticed then. I apologize for the trouble this has caused. I'll do the necessary backouts.

(In reply to Mike de Boer [:mikedeboer] from comment #2) > We'll need to backout the offending changeset, I'm afraid. Moving React to > toolkit is not applicable at this time; Please can you explain the reasons for this, or what discussions have already taken place in the past? Maybe it is time to consider changing them?

(In reply to Mark Banner (:standard8) from comment #3) > (In reply to Mike de Boer [:mikedeboer] from comment #2) > > We'll need to backout the offending changeset, I'm afraid. Moving React to > > toolkit is not applicable at this time; > > Please can you explain the reasons for this, or what discussions have > already taken place in the past? As far as I know, no discussion happened and that's the problem. Requesting review from a peer is usually a good way to get this discussion started. At this point backing out the code is a good first step to then start the discussion and re-land later something that has appropriate reviews and tests. Like Mike, I don't see obvious reasons why this couldn't be implemented with simple vanilla JS.

pkerr is actually redoing the about:webrtc page as part of bug 1100508. I think it would make sense to take any discussion of changes to the about:webrtc page there.

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5) > pkerr is actually redoing the about:webrtc page as part of bug 1100508. I > think it would make sense to take any discussion of changes to the > about:webrtc page there. I'll talk to pkerr first and see what his work on bug 1100508 looks like. If we can circumvent duplicating effort that way, I'm for it.

There's no need to back the patch out immediately. We should make React usable for toolkit pages by moving it to toolkit/ per comment 1. This shouldn't have any impact on the ability for anyone to keep that copy of the library up to date (though such updates will need to factor in any potential compatibility impact on all of the consumers of that copy).

I'm, frankly, a little bit disappointed in how this bug has turned out. * Why should React be added to toolkit? To be absolutely clear: when filing this bug my point was not "move React to toolkit", it was "remove the usage of React for about:webrtc". I don't really see what value React adds besides making it harder to hack on this code (as I expressed in bug 1100508, comment 4; aleth also had some good points in bug 1100508, comment 5). * I'm fairly certain that if this was committed by a volunteer it would have been backed out: the code as it stands would not have passed review, and did not undergo the appropriate review. I'm disappointed to see this cutting of corners on established policies. I hope you'll reconsider.

(In reply to Patrick Cloke [:clokep] from comment #8) > * Why should React be added to toolkit? To be absolutely clear: when filing > this bug my point was not "move React to toolkit", it was "remove the usage > of React for about:webrtc". I don't really see what value React adds besides > making it harder to hack on this code (as I expressed in bug 1100508, > comment 4; aleth also had some good points in bug 1100508, comment 5). Given that you are not the person benefiting from the added value (i.e. are not working on or maintaining about:webrtc), this is a fine opinion for you to hold, but I don't think it's worth taking into consideration when weighing the pro/cons. > * I'm fairly certain that if this was committed by a volunteer it would have > been backed out: the code as it stands would not have passed review, and did > not undergo the appropriate review. I'm disappointed to see this cutting of > corners on established policies. This has nothing to do with volunteers vs. employees. If you're concerned about the process violation, backing code out isn't useful. The way to address that is to communicate to ensure that it doesn't happen again, and that communication has already occurred.

I should apologize in advance for the first part of comment 9 - it likely did not convey what I wanted it to. The point I was trying to make was that the specific benefit to the current about:webrtc page of using React is not really the issue here. There are two issues, as I see it: - the toolkit/ dependency on browser/, which we should really fix (thanks for filing this Patrick) - the general attitude towards any "new tool" that is illustrated by comment 2, comment 4, bug 1100508 comment 4, bug 1100508 comment 5. It shouldn't be that hard to add new tools to Mozilla code, particularly when those tools are very commonly used outside of Mozilla and have proven benefits.

I believe all the concerns captured in this bug were addressed in bug 1100508.