New issue
Advanced search Search tips

Issue 803537 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

DevTools: cleanup console viewport correctness and tests

Project Member Reported by l...@chromium.org, Jan 18 2018

Issue description

The DevTools console viewport has several correctness bugs that should be fixed.  Even if they do not noticeably detract from the experience, they prevent new feature development and make it easy to introduce regressions.

Main tasks are:
- Fixing first/lastVisibleIndex()
- Broken tests (see "verifyViewportIsTallEnough" in console-viewport-*.js)
- Stick to bottom behavior when typing in the prompt
- Prevent duplicate items from breaking viewport
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc06068f6874167d45fa01c59330c98b4216208c

commit cc06068f6874167d45fa01c59330c98b4216208c
Author: Erik Luo <luoe@chromium.org>
Date: Tue Jan 23 05:22:39 2018

DevTools: add console viewport visibility test cleanup

Besides minor cleanup, in this CL:
- Viewport selection.js and stick-to-bottom.js tests no longer
  do "verifyViewportIsTallEnough". This sort of test is now covered
  by "console-viewport-indices.js"
- The actual visible indices can be determined in tests with a
  helper that uses getBoundingClientRect()

Bug:  803537 
Change-Id: I7642051b6a5eb358c536ae29568ec9de91a02a8b
Reviewed-on: https://chromium-review.googlesource.com/874778
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531158}
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-control-expected.txt
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-control.js
[add] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-indices-expected.txt
[add] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-indices.js
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-selection-expected.txt
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-selection.js
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-stick-to-bottom-expected.txt
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-viewport-stick-to-bottom.js
[modify] https://crrev.com/cc06068f6874167d45fa01c59330c98b4216208c/third_party/WebKit/Source/devtools/front_end/console_test_runner/ConsoleTestRunner.js

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/103d429ad700ec1912ba2f655a1811e56e3fee6f

commit 103d429ad700ec1912ba2f655a1811e56e3fee6f
Author: Erik Luo <luoe@chromium.org>
Date: Tue Jan 23 09:48:15 2018

DevTools: fix first/lastVisibleIndex() in Console viewport

The first/lastVisibleIndex() methods require the cumulativeHeights to
be correct. Before, calling refresh() would update these heights
before updating DOM, but left it in a possibly stale state.

After this CL, cumulativeHeights is guaranteed to be updated in the
same microtask once refresh() is called.

Bug:  803537 
Change-Id: I96f28dea8584d88a6ee25804714d04a2d61d7f10
Reviewed-on: https://chromium-review.googlesource.com/875160
Commit-Queue: Erik Luo <luoe@chromium.org>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531191}
[modify] https://crrev.com/103d429ad700ec1912ba2f655a1811e56e3fee6f/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/103d429ad700ec1912ba2f655a1811e56e3fee6f/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewport.js

Comment 3 by l...@chromium.org, May 31 2018

Status: Archived (was: Assigned)

Sign in to add a comment