Closed Bug 1615102 Opened 3 years ago Closed 3 years ago

Double-clicking table resizes handles should resize column to fit content

Categories

(DevTools :: Netmonitor, enhancement, P3)

Product:
DevTools
Component:
Netmonitor
Type:
enhancement

Tracking

(firefox76 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 76
Tracking Flags:
Tracking Status
firefox76 --- fixed

People

(Reporter: Harald, Assigned: farooqbckk)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(1 file)

Bug 1615102 - Resize column to fit content. r=Harald
47 bytes, text/x-phabricator-request
Details | Review

Job story: When resizing columns to see content better, I want to quickly double-click the resize handles in the table header, so that I can get the right size without dragging.

Same behavior as to Excel, Finder, Google Sheets.

Could be an intermediate first bug, given that it mostly involved measuring out the DOM content and applying the size.

Bomsy, wdyt?

Flags: needinfo?(hmanilla)
Keywords: good-first-bug

This would be really handy to have.

Flags: needinfo?(hmanilla)

Can I take this issue?

(In reply to Farooq AR from comment #4)

Can I take this issue?

Clicking (or double-clicking) table header also triggers sorting, so we may be we could add the "Resize column to fit data" option in the context menu (which opens by right-clicking the table header)?

(In reply to Farooq AR from comment #5)

(In reply to Farooq AR from comment #4)

Can I take this issue?

Clicking (or double-clicking) table header also triggers sorting, so we may be we could add the "Resize column to fit data" option in the context menu (which opens by right-clicking the table header)?

I think that adding that context menu item is good idea.

Otherwise, we need to distinguish from clicking on a header and in between headers (on the resize handler)

Search for column-resizer DIV element
https://searchfox.org/mozilla-central/rev/cc0f9906505d0ffa0d3a29a80142379e21cb5fb2/devtools/client/netmonitor/src/components/request-list/RequestListHeader.js#552

Honza

Assignee: nobody → farooqbckk
Status: NEW → ASSIGNED

Farooq, did you come up with a measure+resize heuristic that yo are happy with or can we help?

Status: ASSIGNED → NEW
Flags: needinfo?(farooqbckk)

Sorry, I was a bit caught up in academics. This is the workaround I've came up with so far:

https://codepen.io/farooq_ar/pen/jOPByaL

I am not able to fix the scaling down problem.

Flags: needinfo?(farooqbckk)
Attachment #9130355 - Attachment description: Bug 1615102 - Resize column to fit content. r=Harald → Bug 1615102 - Resize column to fit content. r=Harald, honza, bomsy
Attachment #9130355 - Attachment description: Bug 1615102 - Resize column to fit content. r=Harald, honza, bomsy → Bug 1615102 - Resize column to fit content. r=Harald
Pushed by hkirschner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d17023ed99b6
Resize column to fit content. r=Harald,Honza,bomsy

Backed out changeset d17023ed99b6 (Bug 1615102) for causing devtools failures at devtools/client/netmonitor/test/browser_net_column-resize-fit.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=292536169&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=d17023ed99b62b9d0dab6dd244d4fd7e1b85c1cb

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292545315&repo=autoland&lineNumber=15918

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=292545315&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=5daa3dd7c20c587f377a6d32d13373092ab227b3

[task 2020-03-10T21:07:20.893Z] 21:07:20     INFO - Starting test... 
[task 2020-03-10T21:07:20.893Z] 21:07:20     INFO - > Network event progress: NetworkEvent: 0/1, PayloadReady: 1/1, got NetMonitor:PayloadReady for server0.conn23.netEvent23
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - > Network event progress: NetworkEvent: 1/1, PayloadReady: 1/1, got NetMonitor:NetworkEvent for server0.conn23.netEvent23
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - Testing column resize to fit using double-click on draggable resizer
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - Buffered messages finished
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_column-resize-fit.js | Column file has expected size. - Got 12, expected 11
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - Stack trace:
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochikit/content/browser-test.js:test_is:1320
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_column-resize-fit.js:checkColumnsData:72
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_column-resize-fit.js:null:40
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1062
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1097
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:925
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:918
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_column-resize-fit.js | All visible columns cover 100%. - 
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - Testing column resize to fit using context menu `Resize Column To Fit Content`
[task 2020-03-10T21:07:20.894Z] 21:07:20     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_column-resize-fit.js | Column transferred has expected size. - 
[task 2020-03-10T21:07:20.895Z] 21:07:20     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_column-resize-fit.js | All visible columns cover 100%. - 
[task 2020-03-10T21:07:20.895Z] 21:07:20     INFO - Destroying the specified network monitor.
[task 2020-03-10T21:07:20.895Z] 21:07:20     INFO - Wait for completion of all NetworkUpdateEvents packets...
[task 2020-03-10T21:07:20.975Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 19 (0000029268017C00) [pid = 4748] [serial = 61] [outer = 0000000000000000] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:20.976Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 18 (0000029268D80400) [pid = 4748] [serial = 57] [outer = 0000000000000000] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:20.976Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 17 (0000029266660000) [pid = 4748] [serial = 65] [outer = 0000000000000000] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:20.976Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 16 (000002925E8AE400) [pid = 4748] [serial = 64] [outer = 0000000000000000] [url = about:blank]
[task 2020-03-10T21:07:20.976Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 15 (0000029266666000) [pid = 4748] [serial = 69] [outer = 0000000000000000] [url = about:blank]
[task 2020-03-10T21:07:20.976Z] 21:07:20     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 0000029264D98800 == 1 [pid = 4748] [id = {6038ee11-4b91-4a57-9461-9f6ddddbe0d3}] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:21.061Z] 21:07:21     INFO - All pending requests finished.
[task 2020-03-10T21:07:21.080Z] 21:07:21     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 14 (00000292666DB260) [pid = 4748] [serial = 63] [outer = 0000000000000000] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:21.080Z] 21:07:21     INFO - GECKO(4884) | [Child 4748: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 13 (00000292666DB090) [pid = 4748] [serial = 58] [outer = 0000000000000000] [url = http://example.com/browser/devtools/client/netmonitor/test/html_simple-test-page.html]
[task 2020-03-10T21:07:21.080Z] 21:07:21     INFO - GECKO(4884) | console.error: "Task cancelled"
[task 2020-03-10T21:07:23.026Z] 21:07:23     INFO - GECKO(4884) | [Parent 9028: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 47 (000002B03E297800) [pid = 9028] [serial = 131] [outer = 0000000000000000] [url = about:blank]
[task 2020-03-10T21:07:23.026Z] 21:07:23     INFO - GECKO(4884) | [Parent 9028: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 000002B034B0F000 == 14 [pid = 9028] [id = {70493b25-c00d-42bd-b836-e460876b405c}] [url = about:devtools-toolbox]
[task 2020-03-10T21:07:23.112Z] 21:07:23     INFO - Removing tab.
[task 2020-03-10T21:07:23.112Z] 21:07:23     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2020-03-10T21:07:23.130Z] 21:07:23     INFO - Got event: 'TabClose' on [object XULElement].
[task 2020-03-10T21:07:23.135Z] 21:07:23     INFO - GECKO(4884) | [Parent 9028, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorService.cpp, line 182
[task 2020-03-10T21:07:23.135Z] 21:07:23     INFO - GECKO(4884) | [Parent 9028, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorService.cpp, line 182
[task 2020-03-10T21:07:23.140Z] 21:07:23     INFO - GECKO(4884) | [Parent 9028, Main Thread] WARNING: NS_ENSURE_TRUE(weakFrame.IsAlive()) failed: file /builds/worker/checkouts/gecko/layout/xul/nsXULPopupManager.cpp, line 1053
[task 2020-03-10T21:07:23.142Z] 21:07:23     INFO - Tab removed and finished closing
[task 2020-03-10T21:07:23.142Z] 21:07:23     INFO - Leaving test bound
Flags: needinfo?(farooqbckk)

The following test failed: Testing column resize to fit using double-click on draggable resizer
Reason: Column file has expected size. - Got 12, expected 11

The test works on my machine. The new width was 11.25% (~11%) in my case. I am not sure what could be the reason for the changing width on other machines.

Flags: needinfo?(farooqbckk)

(In reply to Farooq AR from comment #12)

The following test failed: Testing column resize to fit using double-click on draggable resizer
Reason: Column file has expected size. - Got 12, expected 11

The test works on my machine. The new width was 11.25% (~11%) in my case. I am not sure what could be the reason for the changing width on other machines.

We've seen this kind of discrepancy in the past when implementing resizable columns in the Network monitor and used Math.round() for expected values.

See e.g. here https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/netmonitor/test/browser_net_headers-resize.js#212
(and there are other places in the same file)

It's ok to use it in your patch too.
As soon as the patch is ready I can push to try to check if it helps.

Honza

Pinging farooq for comment 13, hopefully this helps.

Flags: needinfo?(farooqbckk)

I've changed it back to Math.round
Should we use one of the todo equivalents of test functions for the failing tests? For e.g., using todo_is instead of is ?

Flags: needinfo?(farooqbckk)

Let's accept range from 11 to 13 (I am seeing these results o all tested platforms)
Try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d105f1bc85ee782374bcf6b57a15b16397a978f0&selectedJob=292832252

The other failures are from browser_net_headers-resize.js which is caused by the delay in startDragging

    // This allows for double-click.
    if (Date.now() - this.clickDelay < 300) {
      return;
    }

I think that the following function in the test needs to be fixed now:
(since the dragging doesn't start immediately)
https://searchfox.org/mozilla-central/rev/7d0c94a0e9a9fe1f83553f49b10128567d21709d/devtools/client/netmonitor/test/browser_net_headers-resize.js#143

We need to introduce some delay in the test using:

await wait forTime(300+x)

... to workaround the delay

Honza

Flags: needinfo?(farooqbckk)

Ah gotcha! We could also add another condition:

if (this.clickDelay !== null && Date.now() - this.clickDelay < 300) {
      return;
}

and setting this.clickDelay = null in the constructor. This works as well and the tests pass.
Which way should we go about it?
Thanks for pointing this out! :)

Flags: needinfo?(farooqbckk)

But you need to make sure that this.clickDelay is set somewhere no?
Honza

Flags: needinfo?(farooqbckk)

It is set right after the if block

Flags: needinfo?(farooqbckk)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e588c4bccf1
Resize column to fit content. r=Harald,Honza,bomsy
Blocks: 1622308
Blocks: 1621042
Flags: needinfo?(farooqbckk)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/328c0c069616
Resize column to fit content. r=Harald,Honza,bomsy

https://hg.mozilla.org/mozilla-central/rev/328c0c069616

Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox76: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76

Added a description of this to the Network request columns section of the "Network request list" article; added a little context so it makes sense.

Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.