272059 - use xul preprocessor to check for mac in tree.xml

Status: RESOLVED WONTFIX

(SeaMonkey :: UI Design, enhancement)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: (Av1) patch

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 2

[Serge Gautherie (:sgautherie)]

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 3

[[e-mail]]

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.

Comment 4

[[e-mail]]

(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.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

(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.

Comment 6

[[e-mail]]

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

Comment 7

[Serge Gautherie (:sgautherie)]

(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 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: (Av2) xml comments + whitespace [Checked in: Comment 13]

Comment 9

[Serge Gautherie (:sgautherie)]

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...

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-TK) <tree.xml> ++

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 12

[Mike Connor [:mconnor]]

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.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

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 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 178754 [details] [diff] [review]
(Av2) xml comments + whitespace
[Checked in: Comment 13]

'approval1.8b3=?':
(MAS only) Trivial nits & comments, no risk.

Comment 15

[Serge Gautherie (:sgautherie)]

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

Comment 16

[[e-mail]]

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