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

onPaginationChanged called 4 times when book opens #383

Open
jdempcy opened this issue Mar 16, 2017 · 7 comments
Open

onPaginationChanged called 4 times when book opens #383

jdempcy opened this issue Mar 16, 2017 · 7 comments

Comments

@jdempcy
Copy link

jdempcy commented Mar 16, 2017

This issue is a Bug

Expected Behaviour

I expect the pagination changed event to only be emitted once when the book opens.

Observed behaviour

I see that it is called 4 times when the book is first opened.

Steps to reproduce

  1. Set a breakpoint in openPageInternal, where it calls onPaginationChanged. (Or set a breakpoint within the onPaginationChanged method.)
  2. Refresh the book and notice that when it is first loaded, this method gets called 4 times.

Product

  • Native application (Readium SDK C++)
    • Android and iOS SDK Launchers, and our hybrid apps.

Additional information

At the top you can see where I set the breakpoint in the readium-shared-js_all.js build artifact. Then you can see the 4 call stacks where the onPaginationChanged method gets called 4 times. This causes a flicker that did not formerly occur.

image

@danielweck
Copy link
Member

Sounds like the so-called "resize sensor" is kicking-in several times in a row (outside of the 100ms updatePagination debounce time window) because of font loading, media queries / CSS, etc.
Are you able to reproduce this exact event sequence for all reflowable books?

@olivierkorner

code reference:
https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L865

@jdempcy
Copy link
Author

jdempcy commented Mar 16, 2017

I actually am not able to reproduce this on Android, because every time I refresh from the Chrome Inspector it loses my breakpoints. Strange, it didn't do this before. I will see if I can figure that out.

In the meantime, I'm testing in iOS and finding that, yes, the ReadiumSDK.Events.PAGINATION_CHANGED event is being emitted four times.

I forget about the 100ms debounce time window you mentioned. But it doesn't seem to be working in this case, or maybe they are emitted >100ms apart.

The issue I was looking at when I found this is just why the cover image flickers when I open a book. And it seems it is flickering because of a redraw triggered by something in the initial layout of the book. (It does not flicker when navigating to a different spine item and returning.)

Curiously, the PAGINATION_CHANGED event is emitted 4 times on page load of reflowable books (I've tested 2 so far) but the one FXL book I tested only emits the PAGINATION_CHANGED event twice. And it does not flicker. Perhaps the flickering is caused by something in the columnization.

It looks like in the two reflowable books I've tested, the order is always the same in terms of the call stack (as in the image above). I can keep testing with other reflowable books if that would be useful.

@jdempcy
Copy link
Author

jdempcy commented Mar 16, 2017

Since it only occurs on book open, I can find a workaround where I, for instance, set the iframe opacity to 0.01 on READER_INITIALIZED which is called first, and then increment the counter on every PAGINATION_CHANGED event and change opacity back to 1 on the 4th event. Assuming it does happen consistently like that every time, which is something I can verify. Not the most robust solution but it may take care of this. Or just setting a timeout from READER_INITIALIZED and "hiding" the iframe (opacity 0.01 to make sure it can still be laid out in the DOM) and then setting opacity back to 1 after a certain interval of time.

@rkwright
Copy link
Member

rkwright commented Apr 20, 2017

@jdempcy Any progress on this Jonah? Forward or backward? And do I understand the report correctly that you see the behaviour on iOS but don't know whether it occurs or not on Android?

@danielweck
Copy link
Member

Just a thought: there is currently no logic in the "Resize Sensor" event handler to detect that it fires too many times (as per a predefined ceiling value) within a given time window (some kind of limit for the debouncer and/or throttler). I remember seeing an edge case in a web browser (ReadiumJS cloud reader) where the page would just keep reloading because of the webview control not settling on a stable document height (probably because of image layout passes in column-paginated reflowable mode, and there were font faces too).
Bottom line: it looks like in the end the document height settles (and consequently the Resize Sensor stops firing), but visible excessive flickering is clearly a problem.

@Jayashreetipi
Copy link

Hiii................... I have successfully build the project. some problem i am facing. if i am adding alert message in radium-shared-js_all.js , it is automatically deleting. it is every time creating new.... Can you please help me....

@jdempcy
Copy link
Author

jdempcy commented Jun 29, 2017

PAGINATION_CHANGED is still firing 3 times for us but we have no visible flicker. This could be due to modifications from our own app, however. I have not verified with either the iOS or SDK Launcher apps to see if the flicker is still there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants