Skip to content

[dart migrate] Can't disable a directory with interactive html tool #45326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
esDotDev opened this issue Mar 15, 2021 · 1 comment
Closed

[dart migrate] Can't disable a directory with interactive html tool #45326

esDotDev opened this issue Mar 15, 2021 · 1 comment
Assignees
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). P0 A serious issue requiring immediate resolution

Comments

@esDotDev
Copy link

I'm using dart migrate, and am inside the HTML suggestions page. According to the docs, you can disable a package with the checkbox, but it's not working.
". To opt out a file or directory, click its green checkbox"

I can disable individual files, but not an entire folder, I get this error:

Uncaught TypeError: Cannot read property 'querySelector' of null
    at Object.hl (flutter-app?authToken=GvgdqvwceKs%3D:3464)
    at Object.hl (flutter-app?authToken=GvgdqvwceKs%3D:3465)
    at TD.$1 (flutter-app?authToken=GvgdqvwceKs%3D:7015)
    at vN.$1 (flutter-app?authToken=GvgdqvwceKs%3D:6407)
    at ft (flutter-app?authToken=GvgdqvwceKs%3D:388)
    at HTMLSpanElement.<anonymous> (flutter-app?authToken=GvgdqvwceKs%3D:396)

image

@a-siva a-siva added the area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). label Mar 15, 2021
@stereotype441 stereotype441 added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Mar 16, 2021
@stereotype441 stereotype441 self-assigned this Mar 16, 2021
@stereotype441
Copy link
Member

I spent some time manually de-obfuscating the stack trace. Here's the code in question, from pkg/nnbd_migration/lib/src/front_end/web/migration.dart:

void updateSubtreeIcons(Element element, NavigationTreeDirectoryNode entity) {
  for (var child in entity.subtree) {
    var childNode = element.querySelector('[data-name*="${child.path}"]'); // (1)
    if (child is NavigationTreeDirectoryNode) {
      updateSubtreeIcons(childNode, child); // (2)
      var childIcon = childNode.querySelector(':scope > .status-icon');
      updateIconsForNode(childIcon, entity);
    } else {
      var childIcon = (childNode.parentNode as Element)
          .querySelector(':scope > .status-icon');
      updateIconsForNode(childIcon, child);
    }
  }
}

The top stack entry is at (1) and the next stack entry is at (2). The third stack entry is here (inside the method writeNavigationSubtree):

        statusIcon.onClick.listen((MouseEvent event) {
          toggleDirectoryMigrationStatus(entity);
          updateSubtreeIcons(li, entity); // (3)
          updateIconsForNode(statusIcon, entity);
          updateParentIcons(li, entity);
        });

So I think what has happened is that when the user clicked on the status icon, updateSubtreeIcons tried to use element.querySelector('[data-name*="${child.path}"]') to read the childNode, and got back null. Since the child was a NavigationTreeDirectoryNode, it made a recursive call to itself (passing null for element), and that in turn caused the null reference exception at (1).

So why did the call to element.querySelector return null? I'm guessing the problem is that since @esDotDev is using Windows (I can tell from the C: in their screenshot), the string passed to querySelector contained backslashes. Backslashes passed to querySelector need to be escaped (see https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector#escaping_special_characters).

I'm working on a fix now.

@stereotype441 stereotype441 added P0 A serious issue requiring immediate resolution and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Mar 16, 2021
dart-bot pushed a commit that referenced this issue Mar 16, 2021
This should be helpful in debugging minified stacktraces reported for
the migration tool, such as #45326.

Bug: #45326
Change-Id: I44c697cfdfd6a06e8af3a2592321a1e9156ca929
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/191380
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). P0 A serious issue requiring immediate resolution
Projects
None yet
Development

No branches or pull requests

3 participants