My favorites | Sign in
Project Home Wiki Issues
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 5006: Switching panels does not correctly set the command line auto-completer
1 person starred this issue and may be notified of changes. Back to list
 
Project Member Reported by simon.lindholm10, Nov 26, 2011
What steps will reproduce the problem?
1. Open the HTML panel.
2. Open the small command line.
3. Go to the console panel.
4. Open the command editor.
5. Go back to the HTML panel.
6. Type something in the command line -> there is no auto-completion.

The auto-completer is not correctly set to a small-commandline-completer when switching panels - setMultiLine should probably be called in this case.
This bug appeared from r12337, since the auto-completer is now longer set on command line focus.

Nov 26, 2011
Project Member #1 simon.lindholm10
Test in r12347. The bugs also seems to appear when you never open the console's command line.
Labels: -FBTest-wanted FBTest-available
Nov 26, 2011
#2 sebastia...@gmx.de
When an FBTest is expected to fail, you should mark it with category: "fails". I did that for your test case in r12350.
Nov 28, 2011
Project Member #3 odva...@gmail.com
1) A patch that fixes the bug I introduced at R12337 committed at R12355
2) A patch that fixes the other problem (completion doesn't work in the command line on other panels if the Console panel displays the command editor) committed at R12356

Simon, this fixes your test, but could you please take a look at #2, not sure if this is the best way to fix this.

Thanks for the FBTest!

Honza

Status: Commit
Owner: odva...@gmail.com
Cc: -odva...@gmail.com
Nov 28, 2011
Project Member #4 simon.lindholm10
Yeah, that doesn't seem like a good fix to me.

Setting the single-row completer even for the command editor breaks the invariant of autoCompleter being the completer for the currently open command line, and also means the command editor can't have its own completer.

Even the thing you restored in the first commit is a bit of hack and should ideally not be there.

As I hinted at, I think we should just make sure to call setMultiLine (or some other function we can hook into) when the command line display changes (either by opening or by switching from command editor to another panel).
Nov 29, 2011
Project Member #5 odva...@gmail.com
> As I hinted at, I think we should just make sure to call setMultiLine (or some other
> function we can hook into) when the command line display changes
> (either by opening or by switching from command editor to another panel).
setMutliline is called for me when:

1) Switching between the Command Line and Command Editor
2) Switching among panels

R12362 - appends a tracing in to the method.

> Setting the single-row completer even for the command editor breaks the invariant
> of autoCompleter being the completer for the currently open command line, and also 
> means the command editor can't have its own completer.
Simon, do you have any suggestions for how to properly fix this? You are currently our lord of completions ;-) and I believe you know better what are the requirements here.

Honza

Dec 2, 2011
Project Member #6 simon.lindholm10
Hm, doing things directly in setMultiLine is wrong, since it isn't (and shouldn't be) run when opening the command line popup.

Let's just keep track of the visibility, like attached. I'm a bit unsure about how the code should look though.
5006.patch
7.9 KB   View   Download
Status: Started
Dec 19, 2011
#7 sebastia...@gmx.de
The visibility state "NONE" is just related to the Command Line Popup, so I would expect it to be set there, i.e. commandLine.js should be unrelated to what commandLinePopup.js does.

Regarding setMultiLine() and https://code.google.com/p/fbug/source/browse/branches/firebug1.9/content/firebug/console/commandLine.js?r=12515#580:
It is called on every Command Line initialization, i.e. every page reload and every switching between the panels, which doesn't look correct to me. The patch doesn't change that.
I think setMultiLine() should be replaced by a toggleCommandEditor() function, which really just toggles between Command Line and Command Editor. In Simon's words this would be from state "SMALL" to "LARGE".

Sebastian
Nov 6, 2012
Project Member #8 simon.lindholm10
Cleaned up the code by changing the invariant in https://github.com/firebug/firebug/commit/81c3033e0ccfe9372c291acb362e8507c3734e5c. Be gone, onCommandLineFocus!

Marking fixed because it already is (since a year back).
Status: Fixed
Cc: -sebastia...@gmx.de sebastia...@gmail.com
Sign in to add a comment

Powered by Google Project Hosting