Skip to content
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

Freezes when showing/hiding popover when many annotations overlap #520

Closed
wjt opened this issue Apr 30, 2015 · 1 comment
Closed

Freezes when showing/hiding popover when many annotations overlap #520

wjt opened this issue Apr 30, 2015 · 1 comment

Comments

@wjt
Copy link
Contributor

wjt commented Apr 30, 2015

In an application I'm working on which uses Annotator, it's common to have many intersecting annotations for some piece of text. Mousing over that text often causes the page to freeze after the popover is first displayed. Here's an extreme example with 150 annotations for the same word – it is using annotator master, but I had the same problem with 1.2.x.

What seems to be happening is: there are many nested <span class="annotator-hl"> elements surrounding the text in question. Mousing over the word triggers Viewer._onHighlightMouseover once for each span; the done callback for the hide timer fires many times, repeatedly re-drawing the same popover. checkOrientation shows up very prominently in a profile, but I feel like the real problem is doing the same work 150 times, not that the work itself is expensive!

I'm working around this in my app by just adding event.stopPropagation() to the mouseover callback (PR to follow) but I don't think that's a viable solution in general: if the page being annotated has its own mouseover behaviours, they'd be broken. (In my case, I control the page, so it's not a problem.)

wjt added a commit to wjt/annotator that referenced this issue Apr 30, 2015
See GH issue openannotation#520.

This is problematic because it will break mouseover events on the
highlight's parent, but it does make the page not freeze when you mouse
in and out of a chunk of text with hundreds of annotations.  See
<http://willthompson.co.uk/misc/annotator-freeze-on-mouseout/> for an
example without this patch.

One alternative would be to use a single span.annotation-hl with
multiple annotation-ids where currently they would be nested. ie,
transform this:

```html
<span class="annotation-hl" data-annotation-id="123">
  <span class="annotator-hl" data-annotation-id="456">
    ...
      Foo
    ...
  </span>
</span>
```

into this:

```html
<span class="annotation-hl" data-annotation-id="123 456 ...">
  Foo
</span>
```

Another alternative would be to stick with the current structure but
make the .done() callback inside _onHighlightMouseover smarter about not
repeating itself.
wjt added a commit to wjt/annotator that referenced this issue May 25, 2015
See GH issue openannotation#520.

If you have a chunk of text with hundreds of annotations at the same
spot, the markup looks like this:

```html
<span class="annotation-hl">
  <span class="annotator-hl">
    ...
      Foo
    ...
  </span>
</span>
```

Mousing over the text fires this event once per `<span>`; so `.load()`
would be called once per `<span>`, each time with the full set of
annotations. `.load()` is surprisingly expensive --
`Widget.checkOrientation` seems costly -- and so in my test case with
150 annotations on the same block of text, the browser would freeze for
~10 seconds.

You might notice that `_onHighlightMouseover` does this:

```javascript
var annotations = $(event.target)
    .parents('.annotator-hl')
```

and wonder whether it's safe to only call it from the outermost event handler.
It is, because `event.target` is the *innermost* `<span>`.

(I tried changing the highlighter to use a single `<span>` for all
annotations covering a piece of text, but it's surprisingly fiddly to
get right if you have annotations which intersect but are not
identical.)
wjt added a commit to wjt/annotator that referenced this issue May 25, 2015
See GH issue openannotation#520.

This is problematic because it will break mouseover events on the
highlight's parent, but it does make the page not freeze when you mouse
in and out of a chunk of text with hundreds of annotations.  See
<http://willthompson.co.uk/misc/annotator-freeze-on-mouseout/> for an
example without this patch.

One alternative would be to use a single span.annotation-hl with
multiple annotation-ids where currently they would be nested. ie,
transform this:

```html
<span class="annotation-hl" data-annotation-id="123">
  <span class="annotator-hl" data-annotation-id="456">
    ...
      Foo
    ...
  </span>
</span>
```

into this:

```html
<span class="annotation-hl" data-annotation-id="123 456 ...">
  Foo
</span>
```

Another alternative would be to stick with the current structure but
make the .done() callback inside _onHighlightMouseover smarter about not
repeating itself.
wjt added a commit to wjt/annotator that referenced this issue Jun 19, 2015
wjt added a commit to wjt/annotator that referenced this issue Jun 19, 2015
See GH issue openannotation#520.

If you have a chunk of text with hundreds of annotations at the same
spot, the markup looks like this:

```html
<span class="annotation-hl">
  <span class="annotator-hl">
    ...
      Foo
    ...
  </span>
</span>
```

Mousing over the text fires this event once per `<span>`; so `.load()`
would be called once per `<span>`, each time with the full set of
annotations. `.load()` is surprisingly expensive --
`Widget.checkOrientation` seems costly -- and so in my test case with
150 annotations on the same block of text, the browser would freeze for
~10 seconds.

(I tried changing the highlighter to use a single `<span>` for all
annotations covering a piece of text, but it's surprisingly fiddly to
get right if you have annotations which intersect but are not
identical.)
wjt added a commit to wjt/annotator that referenced this issue Jun 19, 2015
@tilgovi
Copy link
Member

tilgovi commented Jul 2, 2015

Should be fixed by #521

@tilgovi tilgovi closed this as completed Jul 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants