Using --splash breaks IME input

VERIFIED FIXED in mozilla1.2alpha

Status

()

defect
VERIFIED FIXED
18 years ago
9 years ago

People

(Reporter: kazhik, Assigned: bryner)

Tracking

({inputmethod, intl, relnote})

Trunk
mozilla1.2alpha
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

18 years ago
 
(Reporter)

Comment 1

18 years ago
Input method can't be turned on by Shift + Space if Mozilla is launched
with -splash option.

Comment 3

18 years ago
gdk_init() of nsNativeAppSupportGtk.cpp is the first connection
to GDK when -splash is specified. so I believe we need to call
gdk_set_locale() to enable XIM.

Comment 4

18 years ago
katakai-san: can you work on this?  I am no help on Linux platform.
Assignee: yokoyama → katakai

Comment 5

18 years ago
OK, I'll take this.
Status: NEW → ASSIGNED
That should really be a call to gtk_set_locale().  And, hey, what's the deal
with the gdk_init() below it?  bryner?  Isn't this your code?
(Assignee)

Comment 7

18 years ago
The patch looks fine (r=bryner).  We should probably also figure out a way to
pass through the command line arguments when we call g[dt]k_init().
Summary: Input Method doesn't work with -slash option → Input Method doesn't work with -splash option
Look at how appshell does it.  It should work for this code as well.

Comment 9

18 years ago
Blizzard,

Thank you very much for review. Actually AppShell uses g*t*k_init()
and g*t*k_set_locale(), not g*d*k_init().

http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsAppShell.cpp#260

Bryner, should we re-write splash codes to gtk?
Right, that's what I said originally - that we should use gtk* functions.  That
way we can set up command line handling properly as well.

Comment 11

18 years ago
nsNativeAppSupportGtk.cpp needs to be re-written with using
gtk_init() and gtk_set_locale() to pass command line arguments
and to call gtk_set_locale() for i18n as the same manner in
Mozilla gtk toolkit.

Bryner,

Can you take this?
Assignee: katakai → bryner
Status: ASSIGNED → NEW
Summary: Input Method doesn't work with -splash option → nsNativeAppSupportGtk.cpp needs to be re-written with gtk
(Assignee)

Comment 12

17 years ago
-> 1.0, nominating for beta1.
Status: NEW → ASSIGNED
Keywords: nsbeta1
Target Milestone: --- → mozilla1.0

Updated

17 years ago
Keywords: intl

Comment 13

17 years ago
nsbeta1- per ADT triage team
Keywords: nsbeta1nsbeta1-
Target Milestone: mozilla1.0 → mozilla1.2
(Assignee)

Comment 14

17 years ago
I'd like this to be reconsidered for beta1+.  If this isn't fixed, --splash will
totally break IME keyboard input.
(Assignee)

Comment 15

17 years ago
back to nsbeta1 so this will get re-triaged.
Keywords: nsbeta1-nsbeta1
(Assignee)

Comment 16

17 years ago
I'd prefer if the bug summary remained a description of what actual problem this
causes.
Summary: nsNativeAppSupportGtk.cpp needs to be re-written with gtk → Using --splash breaks IME input

Comment 17

17 years ago
nsbeta1- per Nav triage team, should add explain in release notes
Keywords: nsbeta1nsbeta1-, relnote
(Reporter)

Comment 18

17 years ago
katakai-san's patch is included in our testing build "wazilla".

http://wazilla.sourceforge.jp/

Nobody has reported a problem.
(Reporter)

Updated

17 years ago
Blocks: 157673

Comment 19

17 years ago
*** Bug 156423 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 20

17 years ago
Comment on attachment 63761 [details] [diff] [review]
patch for proposal

Does gdk_set_locale() have a side effect? If not,
this patch should be checked in as a temporary fix.
I'll file a new bug for replacing gdk* with gtk*.
Attachment #63761 - Flags: review?(bryner)
(Assignee)

Updated

17 years ago
Attachment #63761 - Flags: review?(bryner) → review+
Has anyone tested to make sure that this patch doesn't break command line arguments?

Updated

17 years ago
QA Contact: ruixu → ylong

Comment 22

17 years ago
GDK is a part of GTK+ library, so we can use GDK functions in GTK+
application.Then all we have to do is, change |gdk_init| to |gtk_init|, isn't it?

Comment 23

17 years ago
blizzard:

I tried some commandline arguments i know how they should work,
and i found a problem with splash.
when we start mozilla as this:
    mozilla -splash --display :1 &

mozilla should start on display ':1', but both splash and browser window will
appear on display ':0'. because the first |gdk_init(0, NULL)| call will have
decided gdk or gtk's display.

As i comment above, |gdk_init| (and if it's there, |gdk_set_locale|) in
nsSplashScreenGtk::nsSplashScreenGtk, can be |gtk_init| (and |gtk_set_locale|).
so we should to call these with default arguments(argc, argv) at

http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsAppRunner.cpp#1604

around here.

Comment 25

17 years ago
change:
    call these functions before retrieving nsNativeAppSupportGtk
Attachment #112684 - Attachment is obsolete: true

Updated

17 years ago
Attachment #112730 - Flags: review?(bryner)
(Assignee)

Comment 26

17 years ago
Comment on attachment 112730 [details] [diff] [review]
patch v2: calling gtk_set_locale()/gtk_init() in main.

When --splash is not being used, gtk_init() is normally called from
widget/src/gtk/nsAppShell.cpp.	If we're going to call gtk_init from main, that
initialization can be removed as well.	In fact, we could just move all of the
initialization that's in nsAppShell::Create to main, since we're not worrying
about runtime switchable toolkits anymore.
Attachment #112730 - Flags: review?(bryner) → review-

Comment 27

17 years ago
moved also gtk_setlocale/gtk_init from gtk{,2}/nsAppShell.cpp to main().

bryner:
Can we move also installation of gdk colormap<*1>, gle_init<*2> and
GdkRGB initialization<*3> before creating the first GdkWindow(as splash)?
I worry that doing this effects gtk2 build

*1 http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsAppShell.cpp#238
*2 http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsAppShell.cpp#245
*3 http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsAppShell.cpp#249

Updated

17 years ago
Attachment #112730 - Attachment is obsolete: true

Updated

17 years ago
Attachment #112846 - Flags: review?(bryner)

Comment 28

17 years ago
BTW, I haven't tried to build gtk2 build for testing yet,
because I have no gtk2 environment...

Comment 29

17 years ago
changes:
1) made not to call gtk_set_locale for GTK+2, since on GTK+2
  gtk_init calls gtk_set_locale automatically then the call is
  useless.
2) separate GTK+ initialization to static function.

reference:
http://developer.gnome.org/doc/API/2.0/gtk/gtk-General.html#gtk-set-localehttp://developer.gnome.org/doc/API/2.0/gtk/gtk-General.html#gtk-set-locale
Attachment #112846 - Attachment is obsolete: true

Updated

17 years ago
Attachment #112846 - Flags: review?(bryner)

Updated

17 years ago
Attachment #112854 - Flags: review?(bryner)

Comment 30

17 years ago
I investigated a little about comment #27.
we can move some g{t,d}k-related initialization code to main, too.
but this has both a good point and bad points.

good point:
we can install GdkRGB colormap, then '-install' command line option
works to splash screen too.

bad points:
* it'll slow down splash pop-up speed, to insert more code before
  showing splash, then splash has less meaning.
* '#if define' macros will be flooded :(
Comment on attachment 112854 [details] [diff] [review]
patch v4: calling gtk_set_locale()/gtk_init() in main.

>+  // GTK+ initialization moves to main()

Remove these comments.	If you're removed it you've removed it.

> #ifdef MOZ_GLE
>   gle_init (&argc, &argv);

Might as well remove this anyway.

>     // XXX add all of the command line handling
> 
>-    gtk_init(argc, &argv);
>+    // GTK+ initialization moves to main()

Same thing here.  You don't need this comment.

> #ifdef MOZ_WIDGET_GTK
> #include <gtk/gtk.h>
>+
>+static nsresult InitializeGtk(int *argc, char **argv[])
>+{
>+#if !defined(MOZ_WIDGET_GTK2)
>+  // on GTK+2, this'll automatically called.
>+  gtk_set_locale();
>+#endif // !MOZ_WIDGET_GTK2
>+  gtk_init(argc, argv);

This isn't going to compile properly on GTK2.  GTK2 does not imply
MOZ_WIDGET_GTK.  If will have to add seperate sections for MOZ_WIDGET_GTK and
MOZ_WIDGET_GTK2.

Also, this needs to be tested well with dialogs and embedding.

Updated

17 years ago
Attachment #112854 - Flags: review?(bryner)

Comment 32

17 years ago
changes:
1) removed meaningless comments
2) made code gtk1 specific, because MOZ_WIDGET_GTK doesn't affect to gtk2.

blizzard:
Thanks for your advice. BTW, can I remove MOZ_GLE code?
I found "--enable-gle" configure option in Mozilla Unix Build Configurator.

and excuse me, I don't know test with "dialog", what's that?
Attachment #112854 - Attachment is obsolete: true

Updated

17 years ago
Attachment #112877 - Flags: review?(bryner)

Comment 33

17 years ago
I'd tested on cvs trunk mozilla and embed on 2003-01-26

1) test mozilla with a variety of arguments:
   1-a) -h, -help or --help, -v, -version or -version: works
   1-b) -splash or --splash: works fine, I can input Japanese with IM.
   1-c) -splash --display :2 : test a gtk option '--display' with splash
        by using Xnest. also fine. probably other gtk options works as well.
   1-d) -UILocale en-US, -contentLocale en-US: I could test only for "en-US".
        other locales failed for they hasn't installed.
   1-e) -CreateProfile <name>: works. no window open and profile <name> is
        created.
   1-f) -P <name>: works
   1-g) -ProfileManager or -profilemanager: works
   1-h) -SelectProfile: works
   1-i) -ProfileWizard: works
   1-j) -url <url>: works
   1-k) -remote: test all commands at <http://www.mozilla.org/unix/remote.html>
        and all works
   1-l) -width and -height: works
   1-m) -chrome <chrome url>: works
   1-n) -browser, -editor, -mail, -compose, -chat: works

  * these not tested yet. because I don't know how these should work.
   1-o) -silent: works as well as test with 2003-01-28-05-trunk.
        should '-silent' prevent creating new window?
   1-p) -installer 
   1-q) -install
   1-r) -f, -ftimeout, -fwait
   and other command line arguments

2) test TestGtkEmbed:
   run, surf some sites, click link, new window, close it. all green.

Comment 34

17 years ago
I tried this patch for gtk2 build. It works, and passes pre-checkin tests.
I think this patch wouldn't affects gtk2 build.
(Assignee)

Updated

16 years ago
Attachment #112877 - Flags: review?(bryner) → review+

Comment 35

16 years ago
Comment on attachment 112877 [details] [diff] [review]
patch v5: move gtk_set_locale/gtk_init to main

Thanks for your review bryner
Attachment #112877 - Flags: superreview?(blizzard)

Comment 36

16 years ago
bug 186510 fixed. gtk2 build doesn't have original problem, but it also has the
first g{t,d}k_init call with non-proper arguments. 
I think gtk2 build also should call gtk_init in main. because it should be have
the same problem (gtk options are ignored when running with splash).

I'm working on it.

Comment 37

16 years ago
Updated patch. I tried this patch with cvs mozilla on gtk/gtk2 build.
{Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20030217}

I tested gtk and gtk2 build command line arguments as comment #33.
and tried TestGtkEmbed on gtk and gtk2, I have no problem.
Either on gtk and on gtk2, passed pre-checkin test.

changes:
    * moved gtk_init to main() on also gtk2.
    * update for bug 186510 check-in
Attachment #112877 - Attachment is obsolete: true

Updated

16 years ago
Attachment #112877 - Flags: superreview?(blizzard)

Comment 38

16 years ago
Comment on attachment 114662 [details] [diff] [review]
patch v6: move gtk_set_locale/gtk_init to main

sorry please review again bryner
Attachment #114662 - Flags: superreview?(blizzard)
Attachment #114662 - Flags: review?(bryner)
Attachment #114662 - Flags: approval1.3?
Comment on attachment 114662 [details] [diff] [review]
patch v6: move gtk_set_locale/gtk_init to main

sr=blizzard

Seems sane.
Attachment #114662 - Flags: superreview?(blizzard) → superreview+
(Assignee)

Updated

16 years ago
Attachment #114662 - Flags: review?(bryner) → review+

Comment 40

16 years ago
i18n triage team: nsbeta1- 
Comment on attachment 114662 [details] [diff] [review]
patch v6: move gtk_set_locale/gtk_init to main

not for 1.3.
Attachment #114662 - Flags: approval1.3? → approval1.3-

Comment 42

16 years ago
1.4 alpha tree is opened. Is there someone check in this, please?

Comment 43

16 years ago
ftang or nhotta should be able to checkin for you.

Comment 44

16 years ago
could you check in this ftang? please.
(Assignee)

Comment 45

16 years ago
Sorry, didn't realize this was waiting on checkin.  I just committed it.  Thanks
for the patch!
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 46

16 years ago
This is the my first patch which checked-in!
Thanks for your checking in bryner!

Comment 47

16 years ago
*** Bug 151726 has been marked as a duplicate of this bug. ***

Comment 48

16 years ago
Verified fixed on 05-05 trunk build on linux RH8.0.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.