Cannot select single line text in console after bug 960695

RESOLVED FIXED in Firefox 29

Status

RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: Optimizer, Assigned: rcampbell)

Tracking

unspecified
Firefox 29
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
Multiline text selection works just fine, but single line text selection leads to unselecting of text and focusing of input box.

This is a regression due to bug 960695
(Assignee)

Comment 1

5 years ago
affecting mac as well. Thanks for filing.
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Updated

5 years ago
Blocks: 612253
(Assignee)

Comment 2

5 years ago
Created attachment 8366037 [details] [diff] [review]
select

I'll work on the test momentarily.
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
Attachment #8366037 - Flags: review?(mihai.sucan)
Comment on attachment 8366037 [details] [diff] [review]
select

Review of attachment 8366037 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: browser/devtools/webconsole/webconsole.js
@@ +576,3 @@
>       */
> +    this._addMessageLinkCallback(this.outputNode, () => {
> +      this.jsterm.inputNode.focus();

You might want to keep the e.target.nodeName checks.
Attachment #8366037 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 8366697 [details] [diff] [review]
select

added a test. All console tests pass locally.

Tweaked _addMessageLinkCallback a little to clear out previous _startX, _startY entries. Moved callback handlers to arrow functions.

Needed one executeSoon for the final selection test.
Attachment #8366037 - Attachment is obsolete: true
Attachment #8366697 - Flags: review?(mihai.sucan)
(Assignee)

Updated

5 years ago
Blocks: 962531
Comment on attachment 8366697 [details] [diff] [review]
select

Review of attachment 8366697 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks!

::: browser/devtools/webconsole/test/browser_console_click_focus.js
@@ +16,4 @@
>    browser.removeEventListener("DOMContentLoaded", testInputFocus, false);
>  
>    openConsole().then((hud) => {
> +    waitForMessages({

nit: this test is growing. Task.spawn() would help a lot with keeping it cleaner / less convoluted.

(no need to change the test now - just for reference, for future tests)

@@ +59,2 @@
>  
>        finishTest();

Please call finishTest() in the executeSoon() function. ok() can become undefined if finish() is called too early.
Attachment #8366697 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 6

5 years ago
this thing just breaks the network popups. Going to have to revert...
(Assignee)

Comment 7

5 years ago
Created attachment 8368627 [details] [diff] [review]
select

ended up copying the addMessageLinkCallback. The preventDefault() was preventing popups from being triggered over anchors. Should be fixed now, but the unittest is still failing on the drag test. Need to debug it.
Attachment #8366697 - Attachment is obsolete: true
Attachment #8368627 - Flags: review?(mihai.sucan)
(Assignee)

Comment 8

5 years ago
Created attachment 8368671 [details] [diff] [review]
select

sorry. Updating this with a better-working patch. The unittest has the drag test disabled (via todo) for now because it's just not working. Tips welcome.
Attachment #8368627 - Attachment is obsolete: true
Attachment #8368627 - Flags: review?(mihai.sucan)
Attachment #8368671 - Flags: review?(mihai.sucan)
(Assignee)

Comment 9

5 years ago
This is failing locally for some reason with this patch:

 2:22.04 
 2:22.05 INFO TEST-START | Shutdown
 2:22.05 Browser Chrome Test Summary
 2:22.05 	Passed: 2150
 2:22.05 	Failed: 3
 2:22.05 	Todo: 1
 2:22.05 
 2:22.05 *** End BrowserChrome Test Results ***
 2:22.05 JavaScript strict warning: resource://gre/modules/Log.jsm, line 484: reference to undefined property message.level
 2:22.65 INFO | runtests.py | Application ran for: 0:02:18.501832
 2:22.65 INFO | zombiecheck | Reading PID log: /var/folders/6y/zq6nzsqx5zn7929y373p0h8r0000gn/T/tmpmBlJx1pidlog
 2:22.66 WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
 2:22.66 runtests.py | Running tests: end.
 2:22.67 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_console_dead_objects.js | Test timed out
 2:22.67 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_console_dead_objects.js | leaked window property: foobarzTezt
 2:22.67 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_console_dead_objects.js | Found a devtools:webconsole after previous test timed out
(Assignee)

Comment 10

5 years ago
and no failure this time...
Duplicate of this bug: 965042
(Assignee)

Comment 12

5 years ago
Comment on attachment 8368671 [details] [diff] [review]
select

carrying review over. Mihai gave me a verbal r+ in IRC last night.
Attachment #8368671 - Flags: review?(mihai.sucan) → review+
https://hg.mozilla.org/mozilla-central/rev/41ac71fd705e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.