First visible locator (EPUBS) improvements #99
Conversation
…rty to be considered
…to remove/add custom properties in HtmlElements, version 3.6.9 should be able to manage the namespaces
chocolatkey
left a comment
chocolatkey
left a comment
There was a problem hiding this comment.
Looks good, simplifying the CSS selectors is probably a good idea regardless of these bugs for cross-compatibility across reading systems and web engines
| if (display != "block") { | ||
| // Added list-item as it is a common display property for list items | ||
| // TODO: Check if there are other display properties that should be ignored/considered | ||
| if (display != "block" && display != "list-item") { |
There was a problem hiding this comment.
@oscar-rivera-demarque can you explain why we can only allow block and and list-item as the display values? For example, why not inline-block?
There was a problem hiding this comment.
@chocolatkey well, that was actually the question I had about why the display block was necessary here? (As it was working with that validation before).
I'm not sure what we're validating using the display != "block" line. I assumed it's issued because at some point the elements had the display behavior changed based on if they were loaded/allowed, or not. But this time I was not able to confirm if that was the expectation.
That's why I just simply added the display list-item, as a new allowed property, so that way I don't interfere in the current working behavior, but for sure we can review if maybe this check is not even necessary.
We have detected some different publications where the first visible locator is not working as expected. Doing an investigation, most of the cases are related to a crash in the usage of "cssSelectorGenerator" method. Additionally, there is another case covered here, when a publication has a list which is long enough to be shown entirely in the current pages.
Cases explanation:
In the first case, I've detected that, when using TTS for some publications in which the elements have custom properties (eg. xml:lang), the css selector process will crash, so the first visible locator will not be in the actual current page, but in the page in which the latest known element is.
1a- Current behavior in Sept comme setteur publication:
Notice that even if I try to start in the page 46, it will go back to the page 40 (the beginning of the current chapter).
Screen.Recording.2025-01-10.at.3.04.44.PM.mov
1b- New behavior in Sept comme setteur publication:
As now the cssSelectorGenerator is not crashing, it's able to get the right element to start.
Screen.Recording.2025-01-10.at.3.05.14.PM.mov
In the second case, the publication I found is a little bit special, most of the content is wrapped into an UL element.
This situation make the whole list of elements to act as list items, and that's why the current
"display: block"validation is not enough to detect the right element to be selected as the first visible element.2a- Current behavior in CRAAV Therapy publication:
Notice that I'm starting in the page 72, but as most of the content is ignored, the "first visible locator" will be the beginning of the UL (in page 5)
Screen.Recording.2025-01-10.at.3.09.53.PM.mov
2b- New behavior in CRAAV Therapy publication:
As now the elements are not ignored, the first visible locator is actually getting the right one.
Screen.Recording.2025-01-10.at.3.10.28.PM.mov