since people can change navigator.platform, it's unreliable (this is due to bug 166395). so, tree.xml should not depend on its value. we can use the preprocessor for that.

this patch does: - add some deprecation comments per bug 202393 (as seen in firefox's tree.xml) - fixes indentation of two var decls - makes this file identical to firefox's tree.xml - and does what the bug summary says, of course ;)

Comment on attachment 167227 [details] [diff] [review] (Av1) patch Christian: Currently, in addition to this patch, Xpfe has |modifiers="control|, and Toolkit has |modifiers="accel|...

Comment on attachment 167227 [details] [diff] [review] (Av1) patch Contrary to comment #0 navigator.platform is reliable in chrome, but feel free to add in XML comments and fix up the whitespace.

(In reply to comment #2) >Xpfe has |modifiers="control|, and Toolkit has |modifiers="accel|... I couldn't get a consensus from the Mac users I am aware of as to whether the Mac should use control or accel for these bindings; they just use the mouse.

(In reply to comment #3) > (From update of attachment 167227 [details] [diff] [review] [edit]) > Contrary to comment #0 navigator.platform is reliable in chrome I want to note that by the time I wrote comment 0, it was not reliable.

Well, you caught the window of opportunity between 22 July and and 7 Dec...

(In reply to comment #3) > (From update of attachment 167227 [details] [diff] [review] [edit]) > navigator.platform is reliable in chrome, but feel free > to add in XML comments and fix up the whitespace. Then, this change is (now) unneeded; yet, isn't it possible/enough/better to have it the #ifdef way instead of the JS way !? (In reply to comment #4) > (In reply to comment #2) > >Xpfe has |modifiers="control|, and Toolkit has |modifiers="accel|... > I couldn't get a consensus from the Mac users I am aware of as to whether the > Mac should use control or accel for these bindings; they just use the mouse. Well, since our purpose is to synchronize the toolkit(s), Can we agree that it won't cause any/much harm to move to accel, or shall we ask to revert FireFox ? (In fact, same issue with |pageUpOrDownMovesSelection|...)

Comment on attachment 178754 [details] [diff] [review] (Av2) xml comments + whitespace [Checked in: Comment 13] Christian, Neil: This new patch gets us even "farther" from the Toolkit version :-/ See my comment 7...

yeah, that's true. I won't resolve the bug when checking in the patch.

Av2 port to Toolkit Mike, the use of the preprocessor seems to be uneeded anymore. Christian, I remove the extra blank line from TK, instead of adding it to MAS, since the other "blocks" don't have it either... (this will "obsolete" your patch, if Mike r+ this one) Mike, Neil, we need to agree on one (common) way or the other...

Comment on attachment 178811 [details] [diff] [review] (Bv1-TK) <tree.xml> ++ is there a practical reason to use !/Mac/.test(navigator.platform) instead of hardcoding the property value per-platform? Its just a useless check that we know the answer to. Same goes for converting the comments. I'd rather pay the miniscule price of preprocessing a file at build time than have a useless check at runtime.

Checking in xpfe/global/resources/content/bindings/tree.xml; /cvsroot/mozilla/xpfe/global/resources/content/bindings/tree.xml,v <-- tree.xml new revision: 1.46; previous revision: 1.45 done -> default owner for further syncing of these files

Comment on attachment 178754 [details] [diff] [review] (Av2) xml comments + whitespace [Checked in: Comment 13] 'approval1.8b3=?': (MAS only) Trivial nits & comments, no risk.

Filter "spam" on "guifeatures-nobody-20080610".

(In reply to comment #12) >the minuscule price of preprocessing a file at build time ...is more than you credit.