Closed
Bug 400671
Opened 17 years ago
Closed 17 years ago
Location bar ignores entered url and uses autocomplete selected url instead
Categories
(Firefox :: Address Bar, defect, P2)
Firefox
Address Bar
Tracking
()
VERIFIED
FIXED
Firefox 3 beta3
People
(Reporter: mossop, Assigned: moco)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
1.23 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
1. Start typing in the url bar so autocomplete pops up some suggestions. 2. Use the down arrow keys to select one. 3. Use the mouse and click into the url bar, click once to remove hte autocomplete and leave the cursor at the end then click again to move the cursor to somewhere other than the end of the url. 4. Type some changes then hit enter. At this point the url reverts to that selected in step 2 ignoring the changes made.
Flags: blocking-firefox3?
Comment 1•17 years ago
|
||
I've been hit by this for a while, and it's one of the most annoying things I have to deal with currently. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007102204 Minefield/3.0a9pre
OS: Mac OS X → All
Hardware: PC → All
Comment 2•17 years ago
|
||
After Step 2 you lost me, but maybe you mean bug 376868 ?
Reporter | ||
Comment 3•17 years ago
|
||
(In reply to comment #2) > After Step 2 you lost me, but maybe you mean bug 376868 ? That appears to be about the form fill autocomplete, not the url bar. And also 'm reasonably sure that this is a recentish regression but haven't had chance to double check that yet.
Assignee | ||
Comment 4•17 years ago
|
||
I'm able to reproduce this on windows xp, as well.
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 M11
Updated•17 years ago
|
Priority: -- → P3
Comment 5•17 years ago
|
||
(In reply to comment #3) > (In reply to comment #2) > > After Step 2 you lost me, but maybe you mean bug 376868 ? > > That appears to be about the form fill autocomplete, not the url bar. There's also bug 79069. But it's all the same widget, right?
Reporter | ||
Comment 6•17 years ago
|
||
(In reply to comment #5) > There's also bug 79069. But it's all the same widget, right? No that is core which is the old style autocomplete. I believe the toolkit version works differently.
bug 357220, filed on branch, looks pretty similar too.
Assignee | ||
Comment 8•17 years ago
|
||
the steps to reproduce from mossop sound related to bug #406487. I'll work on a fix for that bug, and see if it addresses this one.
Assignee: nobody → sspitzer
Depends on: 406487
Assignee | ||
Comment 9•17 years ago
|
||
> I'll work on a fix for that bug, and see if it addresses this one. The fix for bug #406487 does not address this bug, but this still seems related.
Status: NEW → ASSIGNED
Comment 10•17 years ago
|
||
This bug is a regression from bug 279703 so a few months earlier than the new 2 line autocomplete. I lost the bonsai link for the regression window.
Assignee | ||
Comment 11•17 years ago
|
||
ok, I think I have an idea of what is going on. in nsAutoCompleteController::EnterMatch(), the selected index of our pop is >= 0, even though it should be -1 (or maybe we should check if the pop is closed or not?)
Assignee | ||
Comment 12•17 years ago
|
||
while that fix (checking if the popup is open) works, I think the problem is that when I click to close the popup, we don't call ClosePopup() which should call SetSelectedIndex(-1), which would fix this bug.
Assignee | ||
Comment 13•17 years ago
|
||
Assignee | ||
Comment 14•17 years ago
|
||
here's where we hide the window on windows:
gkwidget.dll!nsWindow::Show(int bState=0) Line 1769 C++
gklayout.dll!nsView::SetVisibility(nsViewVisibility aVisibility=nsViewVisibility_kHide) Line 463 C++
gklayout.dll!nsViewManager::SetViewVisibility(nsIView * aView=0x069b6880, nsViewVisibility aVisible=nsViewVisibility_kHide) Line 1651 C++
gklayout.dll!nsMenuPopupFrame::HidePopup(int aDeselectMenu=1, nsPopupState aNewState=ePopupClosed) Line 611 C++
gklayout.dll!nsXULPopupManager::HidePopupCallback(nsIContent * aPopup=0x06197890, nsMenuPopupFrame * aPopupFrame=0x069a6edc, nsIContent * aNextPopup=0x00000000, nsIContent * aLastPopup=0x00000000, nsPopupType aPopupType=ePopupTypePanel, int aDeselectMenu=1) Line 680 C++
> gklayout.dll!nsXULPopupManager::FirePopupHidingEvent(nsIContent * aPopup=0x06197890, nsIContent * aNextPopup=0x00000000, nsIContent * aLastPopup=0x00000000, nsPresContext * aPresContext=0x06ee4598, nsPopupType aPopupType=ePopupTypePanel, int aDeselectMenu=1) Line 1006 C++
gklayout.dll!nsXULPopupManager::HidePopup(nsIContent * aPopup=0x06197890, int aHideChain=1, int aDeselectMenu=1, int aAsynchronous=0) Line 634 C++
gklayout.dll!nsXULPopupManager::Rollup(nsIContent * * aLastRolledUp=0x025360d4) Line 185 C++
gkwidget.dll!nsWindow::DealWithPopups(HWND__ * inWnd=0x0002112a, unsigned int inMsg=513, unsigned int inWParam=1, long inLParam=2818439, long * outResult=0x0012f7a4) Line 7782 C++
gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x0002112a, unsigned int msg=513, unsigned int wParam=1, long lParam=2818439) Line 1240 + 0x19 bytes C++
user32.dll!7e418734()
[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]
user32.dll!7e418816()
user32.dll!7e4189cd()
user32.dll!7e419402()
user32.dll!7e418a10()
gkwidget.dll!nsAppShell::ProcessNextNativeEvent(int mayWait=1) Line 149 C++
gkwidget.dll!nsBaseAppShell::DoProcessNextNativeEvent(int mayWait=1) Line 137 + 0x11 bytes C++
gkwidget.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00c01ed8, int mayWait=1, unsigned int recursionDepth=0) Line 247 + 0xf bytes C++
xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012f990) Line 500 C++
xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00c01ed8, int mayWait=1) Line 227 + 0x16 bytes C++
gkwidget.dll!nsBaseAppShell::Run() Line 154 + 0xc bytes C++
tkitcmps.dll!nsAppStartup::Run() Line 181 + 0x1c bytes C++
xul.dll!XRE_main(int argc=3, char * * argv=0x00bdd880, const nsXREAppData * aAppData=0x00bddc70) Line 3149 + 0x25 bytes C++
firefox.exe!main(int argc=3, char * * argv=0x00bdd880) Line 153 + 0x12 bytes C++
firefox.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C
firefox.exe!mainCRTStartup() Line 403 C
kernel32.dll!7c816fd7()
in nsXULPopupManager::FirePopupHidingEvent(), we fire the onpopuphiding event.
maybe I need an onpopuphiding handler, and on that event, if we have the right event.target, call SetSelectedIndex(-1)?
Assignee | ||
Comment 15•17 years ago
|
||
we already have a popuphiding handler on autocomplete-base-popup! all we need to do is set the selectedIndex to -1 and whamo.
Attachment #292884 -
Attachment is obsolete: true
Attachment #292896 -
Flags: review?
Assignee | ||
Comment 16•17 years ago
|
||
Attachment #292896 -
Attachment is obsolete: true
Attachment #292896 -
Flags: review?
Assignee | ||
Comment 17•17 years ago
|
||
Comment on attachment 292897 [details] [diff] [review] patch note, this will work for rich and non-rich autocomplete, as both have selectedIndex setters.
Attachment #292897 -
Flags: review?(gavin.sharp)
Updated•17 years ago
|
Flags: in-litmus?
Assignee | ||
Updated•17 years ago
|
Keywords: regression
Priority: P3 → P2
Assignee | ||
Comment 18•17 years ago
|
||
Attachment #292897 -
Attachment is obsolete: true
Attachment #293157 -
Flags: review?(gavin.sharp)
Attachment #292897 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 20•17 years ago
|
||
over irc, gavin asked: gavin| was wondering if you had any idea why it was caused by neil's popup rework sspitzerMsgMe I don't know for sure, but if before neil's fix sspitzerMsgMe we called sspitzerMsgMe nsAutoCompleteController::ClosePopup() sspitzerMsgMe directly sspitzerMsgMe that would explain it sspitzerMsgMe as that would call popup->SetSelectedIndex(-1); sspitzerMsgMe popup->SetSelectedIndex(-1); I haven't tested this theory out yet.
Comment 21•17 years ago
|
||
Comment on attachment 293157 [details] [diff] [review] same fix, but add a comment (In reply to comment #20) > gavin| was wondering if you had any idea why it was caused by neil's > popup rework I think I figured this out. Prior to Neil's patch for bug 279703[1], the autocomplete controller was a nsIRollupListener for the popup, and it's nsIRollupListener::Rollup implementation [2], which would have been called when the popup was hidden by a click, calls ClosePopup(), which calls SetSelectedIndex(-1). [1] http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsAutoCompleteController.cpp&branch=&root=/cvsroot&subdir=mozilla/toolkit/components/autocomplete/src&command=DIFF_FRAMESET&rev1=1.57&rev2=1.58 [2] http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp&rev=1.73#667 (This also brings up the fact that the controller no longer needs to implement nsIRollupListener, given the change in [1]. I'll file a new bug on that.)
Attachment #293157 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 22•17 years ago
|
||
fixed. Checking in toolkit/content/widgets/autocomplete.xml; /cvsroot/mozilla/toolkit/content/widgets/autocomplete.xml,v <-- autocomplete.x ml new revision: 1.103; previous revision: 1.102 done gavin, thanks for figuring out the answer to your question and for the review. did you file a new bug about no longer needing to implement nsIRollupListener?
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 23•17 years ago
|
||
(In reply to comment #22) > did you file a new bug about no longer needing to implement nsIRollupListener? I have now: bug 408892.
Mossop: thanks for the easily-transplantable-into-Litmus testcase; in-litmus+ https://litmus.mozilla.org/show_test.cgi?id=5054
Flags: in-litmus? → in-litmus+
Comment 27•16 years ago
|
||
This has not been fixed. I still see this Feb 02 build. Reopen this bug!
You need to log in
before you can comment on or make changes to this bug.
Description
•