Support the "Find" and "Help" keys on Sun keyboard

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
16 years ago
3 months ago

People

(Reporter: simford, Assigned: yuanyi21)

Tracking

(Blocks 1 bug)

Trunk
Sun
Solaris
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Reporter

Description

16 years ago
Steps to reproduce:
1) Press the "Find" key on a Sun keyboard that has that key
2) Press the "Help" key on a Sun keyboard that has that key

Actual reesults:
Nothing

Expected results:
1) Expected the result to be the same as when pressing Ctrl-F (Find in This
Page...".
2) expected the result to be the same as when pressing F1
Reporter

Comment 1

16 years ago
Posted patch patchSplinter Review
Reporter

Updated

16 years ago
Attachment #137978 - Flags: review?(timeless)
Reporter

Comment 2

16 years ago
Hi Timeless:

could you please take a look on this patch?
Thanks!

Comment 3

16 years ago
Comment on attachment 137978 [details] [diff] [review]
patch

it might be the case that the f19 stuff should be in a platform localized file,
but ...
Attachment #137978 - Flags: review?(timeless) → review+

Updated

16 years ago
OS: Linux → Solaris
Hardware: PC → Sun
Reporter

Updated

16 years ago
Attachment #137978 - Flags: superreview?(Henry.Jia)

Updated

16 years ago
Attachment #137978 - Flags: superreview?(Henry.Jia) → superreview+
Assignee

Comment 4

16 years ago
patch checked into 1.7b (trunk).
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

Comment 5

16 years ago
I think this VKF19 stuff needs to use a solaris ifdef using the xul
pre-processor...It shouldn't end up in the chrome used by windows, osx, etc. my
two cents. 
Assignee

Comment 6

16 years ago
mscott, could you give us an example? 

As far as I know, the content and locale directory under xpfe/browser/resource
have 3 platform specific directories - mac, win, unix. Did you mean that we
should put all solaris related changes into the unix directory?

Comment 7

16 years ago
Scott MacGregor wrote:
> I think this VKF19 stuff needs to use a solaris ifdef using the xul
> pre-processor...

This assumes that both X client (Mozilla) and Xserver (Xsun/Solaris) reside on
the same machine - which is NOT true in such cases.
The correct solution would be to enable such stuff for all X11 platforms, not
only Solaris (just think about a Linux/x86 Mozilla displaying on a Xsun/Solaris
Xserver... then those keys should work, too. Using a Solaris-specific |ifdef|
would break that).

Comment 8

16 years ago
This should either be moved to a platform specific XUL overlay file where we put
platform specific bindings or it needs to use the xul pre-processor..

i.e.

#ifdef XP_SOLARIS
  xul for 2nd key binding
#endif 

We shouldn't put platform specific key bindings in xul files shared by all
platforms like we did here I think.

Hopefully that clarifies the comment I was trying to make.

:)

Comment 9

16 years ago
Scott MacGregor wrote:
> This should either be moved to a platform specific XUL overlay file where we 
> put platform specific bindings or it needs to use the xul pre-processor..
> i.e.
>
> #ifdef XP_SOLARIS
>  xul for 2nd key binding
> #endif 

mscott:
Did you read my previous comment (comment #7) ?

|XP_SOLARIS| is __WRONG__.

The keys are specific to the Xserver being used. A mozilla binary running on
Linux can display on a Solaris Xserver and should therefore be able to use this.
This |ifdef| should ONLY disable the code if no X11 code is being build (e.g.
GTK+/Xlib/Qt etc.-ports should have this enabled).

Comment 10

16 years ago
*sigh* I was just illustrating an example of HOW to use the xul pp which was
what I was asked about. "mscott, could you give us an example?"

I don't care what the ifdef is, just as long as these platform key bindings
aren't showing up in windows and OSX versions of the product. Or moved to a unix
bindings file.

Comment 11

16 years ago
mscott:
bryner said on IRC that the XUL preprocessor can use all defined from
"mozilla-config.h". If that works then a simple
|ifdef MOZ_X11| should be the correct way to |ifdef|-out all X11-specific stuff
(again, the Sun keys should be available for all X11 builds, not only for the
Solaris (see examples above - X11 applications are network transparent and
windows can be displayed between different platforms)).

Comment 12

16 years ago
Scott MacGregor wrote:
> *sigh*

... my comment was not thought as offense to anyone... I am just fearing of new
bugs in this area. I am hoping that most of these bugs have been fixed now and I
don't want new ones.

> I was just illustrating an example of HOW to use the xul pp which was
> what I was asked about. "mscott, could you give us an example?"

Those examples sometimes end up 1:1 in the patches... and I don't have my eyes
eveywhere...
Assignee

Comment 13

16 years ago
reopen to address mscott's comments
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 14

16 years ago
->me
Assignee: Simford.Dong → kyle.yuan
Status: REOPENED → NEW
Assignee

Updated

16 years ago
Attachment #142017 - Flags: superreview?(mscott)
Attachment #142017 - Flags: review?(bsmedberg)

Comment 16

16 years ago
Comment on attachment 142017 [details] [diff] [review]
make keybindings platform specific

I'm obviously not a peer of this code, so mscott has to review it, but the
preprocessor stuff looks fine.
Attachment #142017 - Flags: review?(bsmedberg) → review+

Comment 17

16 years ago
Comment on attachment 142017 [details] [diff] [review]
make keybindings platform specific

thanks for going the extra distance on this one!

I'm assuming roland is happy with using MOZ_X11 as the ifdef qualifier with my
sr.
Attachment #142017 - Flags: superreview?(mscott) → superreview+
Assignee

Comment 18

16 years ago
fix checked in.
Status: NEW → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Oh no, I'm going to have to start making chrome to test trivial patches :-(

Comment 20

16 years ago
Neil, would you prefer seamonkey to use an overlay to add this command instead
of leveraging the xul pre-processor for the effected files?

I suspect you are going to eventually be forced to do this anyway as more and
more seamonkey files are going to leverage the xul pre-processor over time...
This patch almost certainly caused bug 236700.

As for the preprocessor, I still maintain that until we have build-time validity
checking working (sending the tree red if the post-processed file is not
well-formed, for example) we should keep its use to a minimum.  Oh, well.
So, would anyone object if I wrote a patch that moved this keybinding into
unix/platformMailnewsOverlay.xul?
Assignee

Comment 23

15 years ago
I'll be glad to see that.

Updated

15 years ago
Attachment #151034 - Flags: superreview?(Henry.Jia)
Attachment #151034 - Flags: review?(kyle.yuan)
Assignee

Updated

15 years ago
Attachment #151034 - Flags: review?(kyle.yuan) → review+

Updated

15 years ago
Attachment #151034 - Flags: superreview?(Henry.Jia) → superreview+
The keybinding is now in the platform overlay.
Damn, I hadn't noticed that you'd also patched editorOverlay.xul all along :-/
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.