Anchoring fails for one annotation on Guardian climate change article

robertknight commented 8 years ago

Steps to reproduce

Visit https://www.theguardian.com/environment/2016/aug/02/environment-climate-change-records-broken-international-report and activate production Chrome extension
Check error console

Anchoring of an annotation containing the body "melting in summer" fails with this error in the console:

es6.promise.js:127 Unhandled promise rejection TypeError: Cannot read property 'nodeType' of undefined
    at Object.Util.getFirstTextNodeNotBefore (chrome-extension://pnnolcckhgcenejopdkchlggkhidmogi/public/scripts/injector.bundle.js?30ac63:4104:14)
    at BrowserRange.Range.BrowserRange.BrowserRange.normalize (chrome-extension://pnnolcckhgcenejopdkchlggkhidmogi/public/scripts/injector.bundle.js?30ac63:4475:24)
    at chrome-extension://pnnolcckhgcenejopdkchlggkhidmogi/public/scripts/injector.bundle.js?30ac63:1366:29
    at chrome-extension://pnnolcckhgcenejopdkchlggkhidmogi/public/scripts/injector.bundle.js?30ac63:1072:24(anonymous function) @ es6.promise.js:127
es6.promise.js:127 Unhandled promise rejection TypeError: Cannot read property 'nodeType' of undefined(…)(anonymous function) @ es6.promise.js:127

Browser

Chrome v52.0.2716.0

Interestingly I don't see this error when visiting the same page with Via.

judell commented 8 years ago

I am investigating this. Another anomaly here, unsure if related or not, is that location order is unexpected. Manu's convention is to annotate the title so his summary appears first in the default location-sorted sidebar.

But here -- https://hyp.is/z7ElAlqIEeaB4898VeRkKg/www.theguardian.com/environment/2016/aug/02/environment-climate-change-records-broken-international-report -- it doesn't. Instead https://hyp.is/92NkkllsEea8JIP9CV7biQ/www.theguardian.com/environment/2016/aug/02/environment-climate-change-records-broken-international-report appears first, but anchors to the caption of the image, several elements below the title.

judell commented 8 years ago

Observations

The badge reports 29 annotations, corresponding to the 29 public annotations visible at https://hypothes.is/api/search?limit=200&uri=https://www.theguardian.com/environment/2016/aug/02/environment-climate-change-records-broken-international-report
The sidebar reports 26 annotations (with orphans-tab on: 25 annotations, 1 orphan)
The sidebar contains 29 annotation cards
There are 26 top-level annotations, 3 replies
document.querySelectorAll('.annotator-hl'), in the document, finds 25 highlights, the missing one is 3A7KdFo-EeaVNa9aFa94Pg
Several repros here -- https://docs.google.com/document/d/1HC48At3OVWrEcEWHTwUEx7B9M-8YO6ysf01SLmQW6OY -- which yield nothing conclusive but show that deleting either of two different annotations will clear the error.

judell commented 8 years ago

TextPositionSelectors don't agree with TextQuoteSelectors

Compare these two annotations:

0hTyrlltEeaVA9fVSqEUdw

                    {
                        "type": "TextPositionSelector",
                        "end": 4446,
                        "start": 4417
                    },
                    {
                        "exact": "more than 50% of its surface.",
                        "prefix": "grate, experienced melting over ",
                        "type": "TextQuoteSelector",
                        "suffix": "\nThe rapid changes in the climat"
                    }

"text" : "Like posted above ... " "user" : leneae101

S8_CyFo_Eea52vcs3J9AVg

                    {
                        "type": "TextPositionSelector",
                        "end": 4431,
                        "start": 4402
                    },
                    {
                        "exact": "more than 50% of its surface.",
                        "prefix": "grate, experienced melting over ",
                        "type": "TextQuoteSelector",
                        "suffix": "\nThe rapid changes in the climat"
                    }

"text": "This is true. .." "user" : "twilamoon"

The TextQuoteSelectors matches exactly, the TextPositionSelectors have the same length (29) but are offset by about half that length.

judell commented 8 years ago

robertknight commented 8 years ago

Fix anchoring error on Guardian Climate Change article

sean-roberts commented 8 years ago

This is very likely related https://via.hypothes.is/http://www.newyorker.com/science/maria-konnikova/being-a-better-online-reader

judell commented 8 years ago

Here's a diff between an annotation that anchors in that maria-konnikova article and one that doesn't:

I know we've suspected that overlapping annotations may be a factor in some of the problems we've been seeing, this seems to point in that direction.

In that case, btw, the result is the same in via and the extension.

judell commented 8 years ago

But here is a minimal version of the above that does not exhibit the problem:

http://jonudell.net/h/anchoring/anchoring-01.html

judell commented 8 years ago

I'll admit I'm grasping at straws, but here is a possibly useful clue. I created a simpler page at http://jonudell.net/h/anchoring/konnikova_03.html and programmatically recreated the above annotations using only TextQuoteSelector:

            "selector": [
                {
                    "exact": "The text you read on a Kindle or computer simply doesn\u2019t have the same tangibility.",
                    "prefix": "rmer grounding in the material. ",
                    "type": "TextQuoteSelector",
                    "suffix": " In new research that she and he"
                }

and

         "selector": [
                {
                    "exact": "he text you read on a Kindle or computer simply doesn\u2019t have the same tangibility.",
                    "prefix": "mer grounding in the material. T",
                    "type": "TextQuoteSelector",
                    "suffix": " In new research that she and he"
                }
            ]

That page exhibits the problem: one anchors, another does not.

Then, taking a copy of that page at http://jonudell.net/h/anchoring/konnikova_04.html, I made those annotations interactively.

In that case, both anchor as expected:

Could this be a compound/cascading thing where, if TextPositionSelector is messed up (for unknown reasons), then the fallback to TextQuoteSelector also fails for overlap-related reasons? (https://github.com/hypothesis/h/issues/3278)

robertknight commented 8 years ago

One other issue I saw on the Guardian article is then activating, de-activating and then re-activating the extension with v0.47.0 of the client:

Uncaught Error: duplicate define: jquery
    at K (app.js:formatted:166)
    at F (app.js:formatted:173)
    at jquery:10269
    at deletedIds (jquery:26)
    at Object.require.jquery (jquery:38)
    at s (_prelude.js:1)
    at s (_prelude.js:1)
    at _prelude.js:1
    at Object.<anonymous> (annotator.js:2)
    at Object.32.jquery (annotator.js:1956)

hypothesis / client