Closed Bug 1239992 Opened 4 years ago Closed 4 years ago

Quick text selection in console focuses the command line and cancels text selection

Categories

(DevTools :: Console, defect, P2)

defect

Tracking

(firefox45 wontfix, firefox46 wontfix, firefox47 wontfix, firefox48 fixed)

VERIFIED FIXED
Firefox 48
Tracking Status
firefox45 --- wontfix
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- fixed

People

(Reporter: arni2033, Assigned: bgrins)

References

(Blocks 1 open bug, )

Details

(Keywords: regression, Whiteboard: [btpp-fix-later])

Attachments

(1 file)

>>>   My Info:   Win7_64, Nightly 46, 32bit, ID 20160114060719
STR:
1. Open this "data:" url
>   data:text/html,<style>a{b:c;d:e}
2. Open console (Ctrl+Shift+K), enable CSS warnings   [now you've got at least 2 lines in console]
3. Select «'b'» (without outer quotes): move mouse pointer before the first ' in that string, then hold left mouse button, move mouse to the right to select «'b'», then release left mouse button
4. Hold Shift
5. Click between ' and d in the sting «'d'» in the next line.

Result:       
 After Step 4 or even after Step 3 (if you performed it quick enough), command line becomes focused
 Text loses focus and therefore all selection is lost

Expectations: 
 Command line should stay unfocused
 Text should be focused to allow user to copy the text in clipboard
Blocks: 1111089
I'm getting tired of these useless "improvements" of good old stuff.
...meanwhile there're thousands of long-standing bugs.

This is regression from bug 960695. Regression range:
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1b52aa569ced&tochange=8f4ecbf938cd
Blocks: 960695
Keywords: regression
Joe, can you help find an assignee?
Flags: needinfo?(jwalker)
This addFocusCallback code looks like the culprit: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/webconsole.js#2632.  So I believe the intention of this is to re-focus the console input any time you click into the console output.  But it's intending to also not re-focus in certain cases (like if text is being selected), and it's buggy.  Maybe instead of tracking mouse movements it could check to see if there is a selection after the click event happens.
See Also: → 1255213
Brian is the right person to be handling this.
Flags: needinfo?(jwalker)
Assignee: nobody → bgrinstead
This is not a recent regression and requires some fiddling to trigger, but it might be an easy fix so worth some time to look into it
Assignee: bgrinstead → nobody
Priority: -- → P2
Whiteboard: [btpp-fix-later]
(In reply to Brian Grinstead [:bgrins] from comment #5)
> This is not a recent regression and requires some fiddling to trigger
"Fiddling", really? It requires to use Console very carefully NOT to trigger it! Here's shorter STR:

1. Select one word in Console, then hold Shift and click on other word
That comment was based off of the STR in the description.  Agreed it's buggy, I'm seeing if it can be simplified / fixed
Simpler logic and seems to behave more as expected.. let's see what Try thinks of it though: https://treeherder.mozilla.org/#/jobs?repo=try&revision=50271b6ad5d3.

arni2033, can you please check either by applying the patch or by downloading a build once the try finishes and let me know if this new behavior is good?
Flags: needinfo?(arni2033)
Duplicate of this bug: 1255213
(In reply to Brian Grinstead [:bgrins] from comment #9)
I chose an option to download build. Just in case, I tested "firefox-48.0a1.en-US.win32.zip" from:
> http://archive.mozilla.org/pub/firefox/try-builds/bgrinstead@mozilla.com-50271b6ad5d31aff25d0ee262a1624bd829bf0bf/try-win32/
I have written a full review on that "focus feature". Points 1-5 have strong relation to this bug.

Good things:
1) I verify that when I select non-empty amount of text, Input_field is NOT focused on 'mouseup',
    just as expected.
2) If I click in some place in the text then hold Shift and click on other place in text, all text 
    between the first and second clicks is selected, i.e. in the middle of this operation focus goes
    to Input_field, but caret position isn't lost. User doesn't care about blinking caret as 
    everything works as expected
3) Everything above works in conjunction with multiple selection (while holding Ctrl)

Bad things:
4) 1255213 probably isn't duplicate, because if I click on empty place below console messages after
    selecting text, nothing happens. I'm not one of those guys who want this magic changing focus,
    but they might expect Input_field to be focused on 'mouseup' in this case.
5) If some text in Input_field was selected, it noticeably blinks in scenario (2).
    But everything _works_ OK as said in (2)
6) If I select a word and try to drag selected text, Input_field is focused on 'mouseup'. But if I
    release mouse over web page area, it doesn't focus Input_field. That is the only workaround
    if I accidentally started dragging text, and it feels quite hacky.

I am against that confusing focus-switching mechanism, but since almost everything works, the compromise is already reached. I guess.
Flags: needinfo?(arni2033)
*  In (1) it doesn't matter how fast and how often I select text - it all works good.
(In reply to arni2033 from comment #11)
> (In reply to Brian Grinstead [:bgrins] from comment #9)
> I have written a full review on that "focus feature". Points 1-5 have strong
> relation to this bug.

Thanks for checking!

> Good things:
> 1) I verify that when I select non-empty amount of text, Input_field is NOT
> focused on 'mouseup',
>     just as expected.
> 2) If I click in some place in the text then hold Shift and click on other
> place in text, all text 
>     between the first and second clicks is selected, i.e. in the middle of
> this operation focus goes
>     to Input_field, but caret position isn't lost. User doesn't care about
> blinking caret as 
>     everything works as expected
> 3) Everything above works in conjunction with multiple selection (while
> holding Ctrl)

OK that's good

> Bad things:
> 4) 1255213 probably isn't duplicate, because if I click on empty place below
> console messages after
>     selecting text, nothing happens. I'm not one of those guys who want this
> magic changing focus,
>     but they might expect Input_field to be focused on 'mouseup' in this
> case.

So I think the main issue in Bug 1255213 was to do with clicking in the empty area when there weren't any messages so nothing would be selected (and it's quite an annoying issue that this patch resolves).  But that's interesting that it's not focusing since I'd expect the click to unselect text.  We can reevaluate if 1255213 should remain closed after this.

> 5) If some text in Input_field was selected, it noticeably blinks in
> scenario (2).
>     But everything _works_ OK as said in (2)

Yes, maybe we should be delaying the 'focus' behavior to wait for a short period of time after the click happens to prevent a flash.  But I think this patch is a big improvement / simplification in various ways so am still in favor of proceeding with this for now.
(5) was rather an observation. If I was one of those who likes that focus changing, my workflow would be: "click literally somewhere in console, then _immediately_ start typing", and I'd rely not only on focusing Input_field but also on re-selecting text in Input_field as fast as possible

So please don't delay it. I usually report blinking when it's not a part of compromise like this one
Attachment #8735638 - Flags: review?(lclark)
Comment on attachment 8735638 [details]
MozReview Request: Bug 1239992 - Focus input field if text is not selected in webconsole output;r=linclark

https://reviewboard.mozilla.org/r/42885/#review39841

The code changes look good to me, so r+.
Comment on attachment 8735638 [details]
MozReview Request: Bug 1239992 - Focus input field if text is not selected in webconsole output;r=linclark

https://reviewboard.mozilla.org/r/42885/#review39843
Attachment #8735638 - Flags: review+
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e14db462d31d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160401030216

In addition to comment 11:
7) It's not possible to select all text in console by Ctrl+A after clicking on a console message.
   To workaround this, you have to perform 'mousedown' -> Ctrl+A -> 'mouseup' instead. Quite hacky.
   I think it's a part of the "compromise" too   v_v
Status: RESOLVED → VERIFIED
I take it back: another workaround is to doubleclick on free place in console first, then press Ctrl+A
To discover it, I had to actually read the patch...
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.