Closed Bug 601440 Opened 9 years ago Closed 9 years ago

Cannot enter Return key in location/search bar when input field was focused

Categories

(Core :: Widget: Cocoa, defect)

1.9.2 Branch
All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
status2.0 --- unaffected
blocking1.9.2 --- .11+
status1.9.2 --- .11-fixed
status1.9.1 --- unaffected

People

(Reporter: eee.dha, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2.11) Gecko/20101001 Firefox/3.6.11
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2.11) Gecko/20101001 Firefox/3.6.11

Pressing return key in location/search bar is captured by some type of website's input fields.

Reproducible: Always

Steps to Reproduce:
1. Open http://www.yahoo.com/
2. Press Command+L
3. Type any URL
4. Press Retrun key
Actual Results:  
Yahoo starts searching.

Expected Results:  
Open the URL entered at step 3.

Works fine:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11pre) Gecko/20100920 Namoroka/3.6.11pre
Not:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11pre) Gecko/20100921 Namoroka/3.6.11pre

Something wrong in
http://hg.mozilla.org/releases/mozilla-1.9.2/pushloghtml?fromchange=bc021a2ec6a9&tochange=c0c261de23ec
This bug doesn't occur on Fx 3.5.14 beta.  It seems that the fix of bug 548480 causes this bug and bug 602161.  And bug 597389 makes it noticeable.
Summary: Cannot enter Return key in location/search bar when input fields opend → Cannot enter Return key in location/search bar when input field was focused
Version: unspecified → 3.6 Branch
Humm, when I backed out bug 597389, it seems to work fine.

TSM uses invalid object???
Status: UNCONFIRMED → NEW
Component: General → Widget: Cocoa
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → cocoa
Version: 3.6 Branch → 1.9.2 Branch
Nakano-san, bug 597389 is yours.  We should consider new fix with bug 597389 (Flash 10.1 regression) + bug 548480 (Top #1 crasher in Thunderbird 3.1 and there is some reports on security device password in Firefox 3.6).
If bug 597389 causes this bug, why isn't this reproduced on 3.5.14? The combination of bug 597389 and bug 548480 is the cause of this bug??
And I have no idea what is happening by the patches.

There are two differences between trunk and branch.

1. Trunk uses new APIs but branch uses ancient API (KeyScript).
2. Trunk sets the state after the focus is completely moved but branch sets the state during focus moving.

I'll try to create a patch.
Um, the #2 isn't matter. Even if I call KeyScript() with delay, this bug cannot be fixed. I guess that this bug isn't new regression, this was hidden by the if statement which was removed by bug 597389.

If we cannot fix this bug easily, we should backout the patch of bug 597389 which is NOT our fault.
Blocks: 597389
blocking1.9.2: --- → ?
status1.9.1: --- → ?
Keywords: regression
Oops, I tested with wrong STR.

Even if I backedout only the patch of bug 548480, I cannot reproduce this bug. So, this is a new regression.

And I retested my patch, then, it can fix this bug! So, I'll post the patch soon.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attached patch Patch v1.0 (obsolete) — Splinter Review
Attached patch Patch v1.0.1 (obsolete) — Splinter Review
This calls KeyScript() (and [NSInputManager currentInputManager]) after focus moving is finished. This works fine for me.
Attachment #482129 - Attachment is obsolete: true
Attachment #482137 - Flags: review?(smichaud)
Attachment #482137 - Flags: review?(m_kato)
Attachment #482137 - Flags: review?(m_kato) → review+
So how much is this going to affect users using the FF 3.6.11 and/or TB 3.1.5 releases?
(In reply to comment #10)
> So how much is this going to affect users using the FF 3.6.11 and/or TB 3.1.5
> releases?

I'm not sure. I guess that the focused native widget's key events go to old focused native widget by the API's bug. So, this regression can be dangerous in some cases.
Comment on attachment 482137 [details] [diff] [review]
Patch v1.0.1

 nsChildView::~nsChildView()
 {
+  if (--gWidgetCount == 0) {
+    nsTSMManager::Shutdown();
+  }

With this code, nsTSMManager::Shutdown() will be called whenever the
number of open windows falls to zero.  But this can happen without
quitting -- for example when the user closes all open windows.

It's probably better to call nsTSMManager::Shutdown() from
nsAppShell::Exit(), just before or after the call to
NS_RemovePluginKeyEventsHandler().

+nsITimer* nsTSMManager::sTimer = nsnull;

The name should probably be more specific.  How about
"sSyncKeyScriptTimer"?
blocking1.9.2: ? → .11+
We need a working, reviewed patch immediately. We need to include this fix in 3.6.11, and our current plan was to ship this Thursday, Oct 14. Obviously that is blown at this point but the clock is ticking.

eee.dha-- thank you for catching this regression before we shipped with it. This would have annoyed a lot of people.
Masayuki won't be up for a couple of hours yet...
Attached patch Patch v2.0Splinter Review
Attachment #482137 - Attachment is obsolete: true
Attachment #482438 - Flags: review?(smichaud)
Attachment #482137 - Flags: review?(smichaud)
Attachment #482438 - Flags: review?(smichaud) → review+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7023412fa93c

I pushed to 1.9.2 branch.

Is there a 1.9.2.11 branch??
Yep, GECKO19211_20100930_RELBRANCH in mozilla-1.9.2. Please land it there as well.
(there are other relbranches in there for TB and SM but I trust they will sort the landings out on their own).
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2cfbdde77508
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 602161
Verified fixed on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11
Status: RESOLVED → VERIFIED
This caused a 2.3% Ts regression on Camino's 10.4 tinderbox (but not on any of our 10.5 tinderboxen).  Is this patch doing some sort of work at startup only on 10.4?
(In reply to comment #22)
> Is this patch doing some sort of work at startup only
> on 10.4?

No, the patch delays an API call for controlling keyboard layout from "during focus moving" to "after focus moved".
You need to log in before you can comment on or make changes to this bug.