hypothesis / client

The Hypothesis web-based annotation client.
Other
634 stars 196 forks source link

Fix annotations highlights not rendered in Firefox 130 #6548

Closed acelaya closed 2 weeks ago

acelaya commented 2 weeks ago

This PR fixes the loading of annotation highlights when loading the client on a PDF document using the bookmarklet in Firefox 130.

This Firefox version comes with a new PDF.js version which defines TextLayer.renderingDone as private, making it impossible to check it from a context other than the object itself.

I realized that this property is set to true only in one place, and right after that, the TextLayer.div object is appended with an element containing the endOfContent class. See https://github.com/mozilla/pdf.js/blob/1ab9ab67eed886f27127bd801bc349949af5054e/web/text_layer_builder.js#L103-L107

Since div is still public, we can achieve the same result by checking if it contains this element. The solution is not perfect, as it couples with internal implementation details of TextLayer, but so was the previous solution, so this should work for now until we can spend more time on the right approach.

Additionally, I have ensured the renderingDone prop is still used if accessible, for older PDF.js versions, and only fall back to the new approach otherwise.

Testing steps

You can follow the same testing steps described in https://github.com/hypothesis/client/pull/6541, but create some annotations/highlights and see that they are rendered in the document.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.43%. Comparing base (4b60a07) to head (5629551). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6548 +/- ## ======================================= Coverage 99.43% 99.43% ======================================= Files 270 270 Lines 10219 10225 +6 Branches 2417 2419 +2 ======================================= + Hits 10161 10167 +6 Misses 58 58 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.