Skip to content

Commit

Permalink
Fix page end cut off when starting with page break
Browse files Browse the repository at this point in the history 
Fixes johnfactotum/foliate#1179
  • Loading branch information
@johnfactotum
johnfactotum committed Mar 20, 2024
1 parent d9da576 commit b0a869b
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion paginator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,13 @@ class View {
if (this.#column) {
const side = this.#vertical ? 'height' : 'width'
const otherSide = this.#vertical ? 'width' : 'height'
const contentSize = this.#contentRange.getBoundingClientRect()[side]
const contentRect = this.#contentRange.getBoundingClientRect()
const rootRect = this.document.documentElement.getBoundingClientRect()

This comment has been minimized.

Sign in to view

Copy link
@aehlke

aehlke Mar 21, 2024

you use this.document here but a few lines below you have if (this.document)

This comment has been minimized.

Sign in to view

Copy link
@johnfactotum

johnfactotum Mar 21, 2024

Author Owner

Good point. I don't think the if does anything. Everywhere else it is assumed that this.document exists at this point.

This comment has been minimized.

Sign in to view

Copy link
@aehlke

aehlke Mar 21, 2024 •
edited
Loading

in my fork, this.document is undefined here, but I couldn't figure out what's different about mine / why after some investigation. was there a change in recent months to ensure this.document is set here?

This comment has been minimized.

Sign in to view

Copy link
@johnfactotum

johnfactotum Mar 21, 2024

Author Owner

I'm probably missing something, but my understanding, from reading the code now, is that expand() is only ever called

  1. At the end of scrolled() and columnize(), which themselves uses this.document, before calling expand(), so the document is already loaded here.
  2. When there is a resize. Obviously, the resize observer can only observe the document after it's loaded, so it should be defined in this case as well.

But this had been the case even in 07a8b76, which is where the check was added, so I'm not sure why I added the check. And testing it now in Foliate, it does appear to work without any problem. But you can probably add a breakpoint or print a trace to see when it was called.

This comment has been minimized.

Sign in to view

Copy link
@johnfactotum

johnfactotum Mar 23, 2024

Author Owner

I think I figured out what's going on. The problem seems to be that the resize observer fires when the iframe is removed, so it only occurs when navigating to a new section. Fixed with b0d7ed4.

This comment has been minimized.

Sign in to view

Copy link
@aehlke

aehlke Mar 23, 2024

Thanks!!
// offset caused by column break at the start of the page
// which seem to be supported only by WebKit and only for horizontal writing
const contentStart = this.#vertical ? 0
: this.#rtl ? rootRect.right - contentRect.right : contentRect.left - rootRect.left
const contentSize = contentStart + contentRect[side]
const pageCount = Math.ceil(contentSize / this.#size)
const expandedSize = pageCount * this.#size
this.#element.style.padding = '0'
Expand Down

0 comments on commit b0a869b

Please sign in to comment.