Why ?
Currently, pagemap is only loaded when rolling, so your change assumes that - but if one day we have pagemap working with pdf or handmade, we may forget about this one. And logically, the test means check for self.ui.pagemap existence before calling its wantsPageLabels() method - so this reads strange. If you ever want to restrict this to rolling, if self.ui.rolling and self.ui.pagemap and self.ui.pagemap:wantsPageLabels() then would feel more logical.

For consistency. We never check for existence of registered submodules, but always have the main fork paging/rolling. This is the only exception.

When (if) the pagemap is added to paging, we will have to grep pagemap and remove if self.ui.pagemap (or, after the above changes, if self.ui.rolling) from everywhere.

Reading if self.ui.pagemap, one may think that the pagemap module appears depending on some conditions (when a user creates custom pagemap, for example).
But the condition is rolling only.

But the condition is rolling only.

You're used to that because of the numerous checks you did for highlights/annotations and the different formats and expected stuff.

Reading if self.ui.pagemap, one may think that the pagemap module appears depending on some conditions

When reading this here, I wouldn't think that. I would just consider it a logical "exists before accessing the method". Not reading it would have me check if it can happen rolling but not pagemap, or why do the pagemap stuff only when rolling, and have me check that indeed, pagemap is there because rolling is there.
I don't need to know here that pagemap depends on rollling.

So, I disagree :) but if you really want it, I would read it better if it were on 2 lines:

if self.ui.rolling then
    if self.ui.pagemap:wantsPageLabels() then

It would still not answer my above comments, but at least, it would not look like an error in "exists before accessing the method".

I erroneously removed if self.ui.pagemap because I remembered that it was a registered module but didn't remember that it was registered by rollling only. CircleCI tests have failed.
I wouldn't have done the mistake if I saw if self.ui.rolling instead.
Anyway, restoring.

This is a condition that based on the diff looks like it'd nearly always be true? They're likely to be equal in practice?

Annotation datetime property is filled on creation. Then, when modifying the annotation - adding a note, changing the boundaries etc, the AnnotationsModified event is sent and the datetime_updated property is set.

But, when the annotation is created, the event is sent too, so the check is false and datetime_updated isn't created.

Naively I'd expect created and updated to be the same at the time of creation (or put another way, queryable), but I haven't followed the annotation developments in-depth.

But, when the annotation is created, the event is sent too

Feels a bit clumsy to check the datetime against now to guess, in a :onAnnotationsModified(), that it is not modifed but created.
Isn't there a cleaner way to handle that ? Not calling onAnnotationsModified if it is... not modified ? :) or passing another arg created=true ?

Done

(You're just moving it around, so I should have asked that ealier :)
Not worth having it as a module local ?
(Dunno how luajit works with that, but creating that small table everytime, with newly created strings then removed, may uses more cpu than if it was long-living - moreover if this function BookList.getBookStatusString() can be caller tons of time when we browse files or collections.)

Done

This is probably too clever btw; I'd overlooked it previously (wrt #13277)

We have 2 dialogs with plural and 2 with singular usages of these strings, if differentiate.

I mean the string:lower(). Hopefully it doesn't matter too much, let's wait and see if anyone complains. :-)