Closed
Bug 406407
Opened 17 years ago
Closed 16 years ago
Accelerators for textEdit should not be affected by keyboard group/level
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta3
People
(Reporter: ivanov, Assigned: ivanov)
References
Details
Attachments
(1 file, 9 obsolete files)
10.06 KB,
patch
|
roc
:
review+
roc
:
superreview+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.8) Gecko/20071022 Ubuntu/7.10 (gutsy) Firefox/2.0.0.8 Build Identifier: all versions (and current trunk too) To accelerate keypress mozilla uses old gtk routine (gtk_bindings_activate), wich doesn't look on active keybord layout (group/level). Reproducible: Always Steps to Reproduce: 1.change the keybord group to non-Latin 2.go to any textEdit (form) 3.try press any shortcut (ctrl+v, for example) Actual Results: Nothings happens Expected Results: Shortcut should be activated This is a part of Bug 69230 (https://bugzilla.mozilla.org/show_bug.cgi?id=69230) Some important information about using gtk_bindings_activate* is there: http://mail.gnome.org/archives/gtk-devel-list/2007-December/msg00003.html Also, some relevant to this issue discussion is on the Bug 69230 page.
Assignee | ||
Comment 1•17 years ago
|
||
Attachment #291061 -
Flags: superreview?
Attachment #291061 -
Flags: review?
Updated•17 years ago
|
Blocks: 69230
Status: UNCONFIRMED → NEW
Component: General → Keyboard: Navigation
Ever confirmed: true
Product: Mozilla Application Suite → Core
Version: unspecified → Trunk
Comment 2•17 years ago
|
||
(In reply to comment #1) > Created an attachment (id=291061) [details] > Fixes the shortcuts of the textEdit (forms and etc). Maybe fixes Bug 399939 > Evgeniy, you should request review and superview for patch from roc. Click the Details link for the attachment. Enter roc in the textbox associated with "review" (analogously for super-review). Also you patch looks like not patch. You've attached something else.
Updated•17 years ago
|
Assignee: general → nobody
QA Contact: general → keyboard.navigation
Summary: It's a part of Bug 69230. Accelerators for textEdit should not be affected by keyboard group/level → Accelerators for textEdit should not be affected by keyboard group/level
Comment 3•17 years ago
|
||
Comment on attachment 291061 [details] Should be removed, I attached wrong file... Sorry. This isn't the right patch (it's not a patch at all). Please attach the CVS-based patch like what you had in attachment 291019 [details] [diff] [review]. Also, request review and superreview from roc@ocallahan.org on the patch.
Attachment #291061 -
Attachment is obsolete: true
Attachment #291061 -
Flags: superreview?
Attachment #291061 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #291061 -
Attachment description: Fixes the shortcuts of the textEdit (forms and etc). Maybe fixes Bug 399939 → Should be removed, I attached wrong file... Sorry.
Attachment #291061 -
Attachment is patch: false
Updated•17 years ago
|
Assignee: nobody → lolkaantimat
Assignee | ||
Comment 4•17 years ago
|
||
Attachment #291065 -
Flags: superreview?
Attachment #291065 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #291065 -
Flags: superreview?(roc)
Attachment #291065 -
Flags: superreview?
Attachment #291065 -
Flags: review?(roc)
Attachment #291065 -
Flags: review?
Assignee | ||
Comment 5•17 years ago
|
||
Sorry, have slept to few time. Done.
Comment 6•17 years ago
|
||
Comment on attachment 291065 [details] [diff] [review] Fixes the shortcuts of the textEdit (forms and etc). Maybe fixes Bug 399939 This patch includes a reversal of the patch in bug 347854. Why is that there? Make sure your patch only includes the fix for this bug and not some other changes. :)
Assignee | ||
Comment 7•17 years ago
|
||
Hm... :) cvs -d :pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot co mozilla/client.mk cd mozilla make -f client.mk checkout MOZ_CO_PROJECT=browser cd ./widget/src/gtk2 cp /programming/ff/mozilla/widget/src/gtk2/nsNativeKeyBindings.* ./ cp /programming/ff/mozilla/widget/src/gtk2/nsWindow.* ./ cvs diff -u8p . > gtk2_bindings.patch The latest CVS-code... I have looked through the patch... Strange. I need to remake it? I will create new diff in a hour :) It may be diffucult not only to make changes, but to diff them too :D
Comment 8•17 years ago
|
||
(In reply to comment #7) > I have looked through the patch... Strange. > I need to remake it? I will create new diff in a hour :) > > It may be diffucult not only to make changes, but to diff them too :D So, the problem is that you made your changes against an older copy of the source code (beta 1, I guess). In your steps outlined in comment #7, you copy the modified files to the CVS location and just diff them. Well, because there are changes to those files in CVS since beta 1 (or whenever you pulled a source tree), you're picking up a reversal of changes made since that time. You should just edit the clean copies from your CVS checkout and create a diff from those.
Assignee | ||
Comment 9•17 years ago
|
||
Attachment #291065 -
Attachment is obsolete: true
Attachment #291070 -
Flags: superreview?(roc)
Attachment #291070 -
Flags: review?(roc)
Attachment #291065 -
Flags: superreview?(roc)
Attachment #291065 -
Flags: review?(roc)
Assignee | ||
Comment 10•17 years ago
|
||
Reed, thanks. Done. I hope this time everything is ok :)
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
I think a better approach here would be to avoid the global variable and add a maybe-null nsEvent* field to nsNativeKeyEvent and add a helper method to nsContentUtils which fills in an nsNativeKeyEvent from an nsIDOMEvent, doing a QueryInterface to nsIPrivateDOMEvent and calling GetInternalNSEvent to fill in the nsEvent*. Then the GDK key event is available in nativeMsg of the nsGUIEvent. You'll need to check that the nsEvent* field is non-null and that the nsEvent has the right type.
Comment 12•17 years ago
|
||
(In reply to comment #11) > QueryInterface to nsIPrivateDOMEvent and calling GetInternalNSEvent to fill in > the nsEvent*. > You'll need to check that the nsEvent* field is non-null and that > the nsEvent has the right type. See here for an example http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsObjectFrame.cpp&rev=1.622&mark=2996-3003#2993 Check also that nativeMsg is non-null.
Assignee | ||
Comment 13•17 years ago
|
||
Roc, I agree with you. Next Friday/Saturday I will try to work with it. Karl, ok, I will.
Assignee | ||
Comment 14•17 years ago
|
||
Sorry, I said I will it this day, but was too busy and tired. I will work with it next weekend. Karl, thanks a lot for the example. I understood the idea very rapidly.
Assignee | ||
Comment 15•17 years ago
|
||
Roc, what about doing it in nsXBLWindowKeyHandler::WalkHandlers? There is nsIDOMEvent* aKeyEvent there, and I think there should be the same way to get nsGUIEvent. Also there is nsNativeKeyEvent variable there (which is initialized in WalkHandlers and should be used in NativeKeyBindings). Working with nsNativeKeyEvent in ContentUtils may be inconvenient, because there is no such variable there, thus nsEvent* field of nsINativeKeyEvent must be static (I didn't found cpp for it, so it may be tricky to define it). But now I look through ContentUtils and static filed in nsINativeKeyEvent.
Assignee | ||
Comment 16•17 years ago
|
||
The same as previous patch, but doesn't use global variable. I don't see any goodway to patch nsContentEvent. As I wrote above nsNativeKeyEventhave't its object there, and also haven't its cpp file to define static variable (which may be used in nsContentEvent). nsXBL* uses nsNativeKeyEvent for some another needs. I've found some places where nsNativeKeyEvent objects are used (and I think that nsNativeKeyBindings uses them after they are defined), but it would be tricky to work with those code. Also the problem is in gtk2 code, and IMHO patching the base-code is not pretty. Patch doesn't fix ctrl+z, but this cause in another place (When I will find it I will open another bug).
Attachment #291070 -
Attachment is obsolete: true
Attachment #292327 -
Flags: superreview?(roc)
Attachment #292327 -
Flags: review?(roc)
Attachment #291070 -
Flags: superreview?(roc)
Attachment #291070 -
Flags: review?(roc)
Assignee | ||
Updated•17 years ago
|
Attachment #292327 -
Attachment description: The patch does the same as previous, bit doesn't use global variable → The patch does the same as previous, but doesn't use global variable
> The same as previous patch, but doesn't use global variable. gEvent is a global variable. My idea was that you would set up the nsEvent* field of nsNativeKeyEvent at the places where we currently construct an nsNativeKeyEvent: http://mxr.mozilla.org/seamonkey/search?string=nsnativekeyevent I don't see any problem with doing that in each of the four locations. In fact we should just move DOMEventToNativeKeyEvent from nsTextControlFrame.cpp to nsContentUtils.cpp and add the nsEvent* setup to that function. Maybe I don't understand what the difficulty is that you're having.
Assignee | ||
Comment 18•17 years ago
|
||
> > The same as previous patch, but doesn't use global variable. > > gEvent is a global variable. No, it's a member of nsNativeKeyBindings class. The definition is: GdkEventKey* nsNativeKeyBindings::gEvent = 0; > My idea was that you would set up the nsEvent* field of nsNativeKeyEvent at the > places where we currently construct an nsNativeKeyEvent: > http://mxr.mozilla.org/seamonkey/search?string=nsnativekeyevent > I don't see any problem with doing that in each of the four locations. In fact > we should just move DOMEventToNativeKeyEvent from nsTextControlFrame.cpp to > nsContentUtils.cpp and add the nsEvent* setup to that function. I've found the code from nsTextControlFrame.cpp, but I thought > Maybe I don't understand what the difficulty is that you're having. that there is no easy way to get GdkEventKey from nsIDOMEvent. I don't know why I decided so. Karl's example show a good way (but I mistaked with the type). Sorry my segfault. I will fix it in a way you've told.
> No, it's a member of nsNativeKeyBindings class.
> The definition is:
> GdkEventKey* nsNativeKeyBindings::gEvent = 0;
OK, it's not in the global namespace, but it's still a "global variable" in the sense that there's only one in the entire program.
Assignee | ||
Comment 20•17 years ago
|
||
> OK, it's not in the global namespace, but it's still a "global variable" in the
> sense that there's only one in the entire program.
Ok :) The main sense that it should be done in another way and it would be done in another way.
Assignee | ||
Comment 21•17 years ago
|
||
Roc, I don't know if I did everything as you told, but it works but with a bug (everything as it was). I attach the patch, so, please, have a look. Also I've found interesting thing. If to comment the string with gtk_bindings_activate* the shortcuts work (only Latin). So... This weekend I will try to make the patch attached work, but I'm not sure it can be done in a way you have told (maybe can, but with many changes). So, I'm waiting for your comments.
+static void + FillNativeEvent(nsIDOMEvent *aDOMEvent, + nsEvent *nativeEvent) +{ + nsCOMPtr<nsIPrivateDOMEvent> privateEvent(do_QueryInterface(aDOMEvent)); + if (privateEvent) + privateEvent->GetInternalNSEvent(&nativeEvent); + else + nativeEvent = 0; +} It doesn't work because you're just setting nativeEvent, which is effectively a local variable here. Make this function GetNativeEvent, taking an nsIDOMEvent* and returning an nsEvent*. DOMEventToNativeKeyEvent would then do aNativeEvent->nativeEvent = GetNativeEvent(...). You'll also need to change the XBL code that sets up nsNativeKeyEvent, because otherwise its nativeEvent field won't be initialized and you'll crash when you look at it in nsNativeKeyBindings::KeyPress. The best way to fix that is to move DOMEventToNativeKeyEvent to nsContentUtils and call it from XBL.
Assignee | ||
Comment 23•17 years ago
|
||
Ok, here it is. I hope everything is ok. Thanks to all for comments and important advices. Sorry for my mistakes and some misunderstanding (especially arguing with Roc). I'm just beginner, but I hope I will be able to fix another things (but maybe only in January, the winter term is going on).
Attachment #292327 -
Attachment is obsolete: true
Attachment #292632 -
Attachment is obsolete: true
Attachment #292760 -
Flags: superreview?(roc)
Attachment #292760 -
Flags: review?(roc)
Attachment #292327 -
Flags: superreview?(roc)
Attachment #292327 -
Flags: review?(roc)
+ const nsGUIEvent *guiEvent = (nsGUIEvent*)aEvent.nativeEvent; + GdkEventKey* gEvent = (GdkEventKey*)(guiEvent->nativeMsg); + + if (aEvent.nativeEvent != 0 && + aEvent.nativeEvent->message == NS_KEY_PRESS && + gEvent != 0) + gtk_bindings_activate_event(GTK_OBJECT(mNativeTarget),gEvent); Use static_cast<> instead of C-style casts here. Note that with this code, if aEvent.nativeEvent is null, we'll have crashed before we check whether it's null in the if statement. The easisest way to fix this is to remove gEvent completely and just write guiEvent->nativeMsg twice (with the cast when you pass it to gtk_bindings_activate_event. Can gtk_bindings_activate_event handle a null event? If not we'd better check that guiEvent->nativeMsg is not null too. + +nsEvent* + nsContentUtils::GetNativeEvent(nsIDOMEvent *aDOMEvent) +{ + nsCOMPtr<nsIPrivateDOMEvent> privateEvent(do_QueryInterface(aDOMEvent)); + nsEvent *nativeEvent; The spacing here is all strange. Get rid of the extra spaces. + nsEvent *nativeEvent; + if (privateEvent) + privateEvent->GetInternalNSEvent(&nativeEvent); + else + nativeEvent = 0; You could write this as if (!privateEvent) return nsnull; nsEvent* nativeEvent; ... GetInternalNSEvent ... return nativeEvent; +PRBool + nsContentUtils::DOMEventToNativeKeyEvent(nsIDOMEvent *aDOMEvent, + nsNativeKeyEvent *aNativeEvent, + PRBool aGetCharCode) More unnecessary space at the start of the function name. Also we put the * next to the type name in this code. + + + + + + + /* static */ Get rid of these blank lines, only one is needed. In nsXBLWindowKeyHandler::WalkHandlers the code that sets the charCode and the modifier keys is no longer needed, DOMEventToNativeKeyEvent does it now. Right? Thanks for your work, I really appreciate your enthusiasm!
Assignee | ||
Comment 25•17 years ago
|
||
Attachment #292760 -
Attachment is obsolete: true
Attachment #292844 -
Flags: superreview?(roc)
Attachment #292844 -
Flags: review?(roc)
Attachment #292760 -
Flags: superreview?(roc)
Attachment #292760 -
Flags: review?(roc)
+ const nsGUIEvent *guiEvent = (nsGUIEvent*)aEvent.nativeEvent; static_cast here + if (aEvent.nativeEvent != 0 && + aEvent.nativeEvent->message == NS_KEY_PRESS && + guiEvent->nativeMsg != 0) Don't need "!= 0" here. Also, can use guiEvent instead of aEvent.nativeEvent. So if (guiEvent && guiEvent->message == NS_KEY_PRESS && guiEvent->nativeMsg) Otherwise good.
Assignee | ||
Comment 27•17 years ago
|
||
(In reply to comment #24) > + GdkEventKey* gEvent = (GdkEventKey*)(guiEvent->nativeMsg); > + > + gEvent != 0 > + gtk_bindings_activate_event(GTK_OBJECT(mNativeTarget),gEvent); > > Use static_cast<> instead of C-style casts here. Ok. > Note that with this code, if aEvent.nativeEvent is null, we'll have crashed > before we check whether it's null in the if statement. You truly right. I didn't think about it (but from this time I will remember to check pointers before '->' or '.'). >The easisest way to fix > Can gtk_bindings_activate_event handle a null event? If not we'd better check > that guiEvent->nativeMsg is not null too. I checked it (gEvent != 0). I've looked through the code: gtk_bindings_activate_event checks only gtkObject (maybe to wright to gtk+-devel?) > The spacing here is all strange. Get rid of the extra spaces. > You could write this as > More unnecessary space at the start of the function name. Also we put the * > next to the type name in this code. All have been done. > > In nsXBLWindowKeyHandler::WalkHandlers the code that sets the charCode and the > modifier keys is no longer needed, DOMEventToNativeKeyEvent does it now. Right? Yes :) I didn't think about it (Oh, we have found the sources of bugs)) ) > Thanks for your work, I really appreciate your enthusiasm! Thanks a lot to you for your help and explanations. It's very important for me. I hope we will work with Bug 407604 :) I will try to make much less mistakes. I have few time before January (winter term), but will nibble the code every free time.
Attachment #292844 -
Attachment is obsolete: true
Attachment #292849 -
Flags: superreview?(roc)
Attachment #292849 -
Flags: review?(roc)
Attachment #292844 -
Flags: superreview?(roc)
Attachment #292844 -
Flags: review?(roc)
+ if (guiEvent->nativeEvent && + guiEvent->nativeEvent->message == NS_KEY_PRESS && You don't want guiEvent-> here. Does this even compile?
Assignee | ||
Comment 29•17 years ago
|
||
Yes. Sorry my absent-mindedness: I forgot to remove .o for this file whiloe checking.
Attachment #292849 -
Attachment is obsolete: true
Attachment #292849 -
Flags: superreview?(roc)
Attachment #292849 -
Flags: review?(roc)
Assignee | ||
Updated•17 years ago
|
Attachment #292853 -
Flags: superreview?(roc)
Attachment #292853 -
Flags: review?(roc)
Comment on attachment 292853 [details] [diff] [review] clear patch 3... hooray!
Attachment #292853 -
Flags: superreview?(roc)
Attachment #292853 -
Flags: superreview+
Attachment #292853 -
Flags: review?(roc)
Attachment #292853 -
Flags: review+
Assignee | ||
Comment 31•17 years ago
|
||
thunderous hurrah!!! :) I've added checkin-needed to the keywords. If I need something else? :)
Keywords: checkin-needed
Updated•17 years ago
|
Attachment #292853 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #292853 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 33•17 years ago
|
||
Reed, and now it's approved. So, checking-in is the last step? And it will in the trunk? May I change the resolution to the "FIXED"?
Comment 34•17 years ago
|
||
(In reply to comment #33) > Reed, and now it's approved. So, checking-in is the last step? And it will in > the trunk? > May I change the resolution to the "FIXED"? It wasn't checked-in yet, but I just did that. Resolving as FIXED now. :) Checking in widget/src/gtk2/nsNativeKeyBindings.cpp; /cvsroot/mozilla/widget/src/gtk2/nsNativeKeyBindings.cpp,v <-- nsNativeKeyBindings.cpp new revision: 1.4; previous revision: 1.3 done Checking in widget/public/nsINativeKeyBindings.h; /cvsroot/mozilla/widget/public/nsINativeKeyBindings.h,v <-- nsINativeKeyBindings.h new revision: 1.3; previous revision: 1.2 done Checking in layout/forms/nsTextControlFrame.cpp; /cvsroot/mozilla/layout/forms/nsTextControlFrame.cpp,v <-- nsTextControlFrame.cpp new revision: 3.273; previous revision: 3.272 done Checking in content/xbl/src/nsXBLWindowKeyHandler.cpp; /cvsroot/mozilla/content/xbl/src/nsXBLWindowKeyHandler.cpp,v <-- nsXBLWindowKeyHandler.cpp new revision: 1.43; previous revision: 1.42 done Checking in content/base/public/nsContentUtils.h; /cvsroot/mozilla/content/base/public/nsContentUtils.h,v <-- nsContentUtils.h new revision: 1.152; previous revision: 1.151 done Checking in content/base/src/nsContentUtils.cpp; /cvsroot/mozilla/content/base/src/nsContentUtils.cpp,v <-- nsContentUtils.cpp new revision: 1.260; previous revision: 1.259 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
Assignee | ||
Comment 35•17 years ago
|
||
Hooray!!! Reed, you made my morning shiny! :) Thanks for checking in :)
Comment 36•17 years ago
|
||
Had to back this out, as it caused 77 test failures on Linux and a Tp regression on Mac. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1197619060.1197622163.16198.gz shows the test failures.
Updated•17 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 37•17 years ago
|
||
(In reply to comment #36) > Had to back this out, as it caused 77 test failures on Linux and a Tp > regression on Mac. Looks like the Tp regression on Mac was unrelated (network issues), so you just need to worry about the 77 test failures on Linux. :)
Assignee | ||
Comment 38•17 years ago
|
||
Hm... Are you sure it's caused by the changes (I was able to have a look only very rapidly and understood nothing - never seen such test before). I have built and tested it by hands - everything was ok (if to be sincere I've tested the code with 3.0b1).
Comment 39•17 years ago
|
||
(In reply to comment #38) > Hm... Are you sure it's caused by the changes (I was able to have a look only > very rapidly and understood nothing - never seen such test before). > I have built and tested it by hands - everything was ok (if to be sincere I've > tested the code with 3.0b1). Yes, the 77 test failures went away when I backed out the patch.
Assignee | ||
Comment 40•17 years ago
|
||
I will try to have a look. The code looks safe. Have you any advices (maybe links)? Run failure tests by hands (and look into their code?). Anywhere, before 29-th December I won't be able to work with it ((
Assignee | ||
Comment 41•17 years ago
|
||
I looked the tests and found some bug in the new-version of nsXBLWindow*.cpp. I don't know if it caused by this, but there are no other changes in the code (just removing the function can't be the problem). Do I need to remove the whole build directory to test it? I removed its .o, but with no results (I mean if it may be linked with some other code into another .o?)
Assignee | ||
Comment 42•17 years ago
|
||
Ok, the problem is: + if (guiEvent && + guiEvent->message == NS_KEY_PRESS && + guiEvent->nativeMsg) + gtk_bindings_activate_event(GTK_OBJECT(mNativeTarget),static_cast<GdkEventKey*>(guiEvent->nativeMsg)); I guess, it's in the if. I've tried if (guiEvent && (guiEvent->message == NS_KEY_PRESS || guiEvent->message == NS_KEY_UP || guiEvent->message == NS_KEY_DOWN || guiEvent->message == NS_CONTROL_CHANGE ) && guiEvent->nativeMsg) If to omit "guiEvent->message ==" application dies But the same. So, what was activated in the gtk_bindings_activate before changes? Not only keys. Of course I will debug it next time, but it's more pleasant to get this information without code-puzzles.
Assignee | ||
Comment 43•17 years ago
|
||
gtk_bindings_activate makes a bit more than gtk_bindings_activate_event. I don't know what exactly, but I thing it's not very important. Using them both works as well as it should (I didn't got tests failures, but I wasn't able to pass all of them). Also I've fixed some things in the nsXBLWindowKeyHandler.cpp.
Attachment #292853 -
Attachment is obsolete: true
Attachment #293428 -
Flags: superreview?(roc)
Attachment #293428 -
Flags: review?(roc)
Attachment #293428 -
Flags: superreview?(roc)
Attachment #293428 -
Flags: superreview+
Attachment #293428 -
Flags: review?(roc)
Attachment #293428 -
Flags: review+
Updated•17 years ago
|
Attachment #293428 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #293428 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 44•17 years ago
|
||
I fixed some comments / indentation on check-in. Checking in widget/src/gtk2/nsNativeKeyBindings.cpp; /cvsroot/mozilla/widget/src/gtk2/nsNativeKeyBindings.cpp,v <-- nsNativeKeyBindings.cpp new revision: 1.6; previous revision: 1.5 done Checking in widget/public/nsINativeKeyBindings.h; /cvsroot/mozilla/widget/public/nsINativeKeyBindings.h,v <-- nsINativeKeyBindings.h new revision: 1.5; previous revision: 1.4 done Checking in content/base/src/nsContentUtils.cpp; /cvsroot/mozilla/content/base/src/nsContentUtils.cpp,v <-- nsContentUtils.cpp new revision: 1.262; previous revision: 1.261 done Checking in content/base/public/nsContentUtils.h; /cvsroot/mozilla/content/base/public/nsContentUtils.h,v <-- nsContentUtils.h new revision: 1.154; previous revision: 1.153 done Checking in content/xbl/src/nsXBLWindowKeyHandler.cpp; /cvsroot/mozilla/content/xbl/src/nsXBLWindowKeyHandler.cpp,v <-- nsXBLWindowKeyHandler.cpp new revision: 1.45; previous revision: 1.44 done
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Assignee | ||
Comment 45•17 years ago
|
||
Fine! Thanks to all for your help! It's time to play with Bug 407604 and to try to make a Christmas present to mozilla trunk :D
Comment 46•16 years ago
|
||
You forgot to include the change in nsTextControlFrame.cpp when you updated your patch. This has caused bug 409570. nit: in widget/src/gtk2/nsNativeKeyBindings.cpp you should try not to go beyond column 80, some comments are way beyond that.
Comment 47•16 years ago
|
||
(In reply to comment #46) > You forgot to include the change in nsTextControlFrame.cpp when you updated > your patch. This has caused bug 409570. Should I just reland the nsTextControlFrame.cpp change? It's pretty small, and it's already been reviewed by roc once. Will that fix the regression completely?
Comment 48•16 years ago
|
||
(In reply to comment #47) > Should I just reland the nsTextControlFrame.cpp change? It's pretty small, and > it's already been reviewed by roc once. Will that fix the regression > completely? Yes, that will fix the regression. I tried do do that and I don't get the crash in bug 409570 any more.
Comment 49•16 years ago
|
||
(In reply to comment #48) > Yes, that will fix the regression. I tried do do that and I don't get the crash > in bug 409570 any more. Ok, relanded. Checking in layout/forms/nsTextControlFrame.cpp; /cvsroot/mozilla/layout/forms/nsTextControlFrame.cpp,v <-- nsTextControlFrame.cpp new revision: 3.275; previous revision: 3.274 done
Status: REOPENED → RESOLVED
Closed: 17 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 50•16 years ago
|
||
I'm very sorry, that I've made such mistake.
Updated•5 years ago
|
Component: Keyboard: Navigation → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•