matatk / landmarks

Allows you to navigate a web page via WAI-ARIA landmarks, using the keyboard or a pop-up menu
http://matatk.agrip.org.uk/landmarks/
MIT License
125 stars 7 forks source link

Mutation Observer does not seems to work correctly #458

Closed cbou closed 3 years ago

cbou commented 3 years ago

On my app the roles that are set with JavaScript later after the page load are not detected by the plugin.

I look at the code and found out that you use a MutationObserver which should be used in this case.

Actually this does not seems to work. Following is a HTML page which define a landmark after 2 seconds.

The landmark won't be detected by the plugin. If you click on another tab and then back to this dummy page then the landmark will be find.

Is that the expected behaviour?

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Hello World! Site Title</title>
  </head>
  <body>
    <div class="foo">
      <h1>Hello World!</h1>
    </div>
    <script>
      setTimeout(() => {
        document.querySelector('.foo').setAttribute("aria-label", "bar")
        document.querySelector('.foo').setAttribute("role", "banner")
        alert('Landmarks added')
      }, 2000)
    </script>
  </body>
</html>
matatk commented 3 years ago

Thanks for your clear bug report and test page. I can reproduce the issue using Firefox (93) and Chrome (95). Interestingly, the "Mutation information" section in the DevTools > DOM Inspector > Landmarks pane shows that no mutations were detected. The reason why the landmark shows up when you move back to the page is that Landmarks will always run a scan shortly after a page becomes visible, in case the page changed whilst it was hidden. (The reason for the delay after the page becomes visible is that you might be switching tabs using the keyboard, to move through several tabs in quick succession.)

I tried making a debug build of the extension to get more info, and it seems that the MutationObserver itself s not firing—but I then tried creating one directly on the test page, and it did fire. I will need to investigate this more to find out the cause. It's likely to be a few days at least before I can do that; I will update this thread with any news.

matatk commented 3 years ago

I made a separate, minimal browser extension to try and track the problem down, and I was able to get it to work with your example code (i.e. the mutations were detected by the extension's content script).

Looks like this problem was introduced in a change to the content script that is causing mutations to only be observed after the page is hidden and then re-shown; quite a big mistake—oh dear! Thanks again for raising this, and providing a clear and minimal example.

I've often thought it would be good to have tests for at least some key behaviours of the extension (to go alongside the existing tests for the accuracy of scans), but the effort of mocking the whole extension environment would be very great, and I haven't figured out a way to test parts in a more isolated fashion. On the plus side, this code changes very rarely, and I put in a lot of logging; will aim to strengthen that and test it for longer in future.

There are a couple of details I'm still working out for this, but I will fix it as soon as possible.