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

'DRY' up the CFI blacklist #381

Open
jccr opened this issue Mar 2, 2017 · 3 comments
Open

'DRY' up the CFI blacklist #381

jccr opened this issue Mar 2, 2017 · 3 comments

Comments

@jccr
Copy link
Contributor

jccr commented Mar 2, 2017

This issue is a Code Cleaning Task

Related issue(s) and/or pull request(s)

#380, #375

Expected

The CFI blacklist to have one single source of truth

Observed

The CFI blacklist is duplicated in the ReflowableView and OnePageView

  • readium-js/readium-shared-js/js/views/one_page_view.js:935
  • readium-js/readium-shared-js/js/views/reflowable_view.js:63

Additional Information

The issues referenced here were caused because one of the cfi blacklist set went out of sync from the other.

@danielweck
Copy link
Member

https://github.com/readium/readium-shared-js/blob/develop/js/views/one_page_view.js#L983

        return new CfiNavigationLogic({
            $iframe: _$iframe,
            frameDimensions: getFrameDimensions,
            visibleContentOffsets: getVisibleContentOffsets,
            classBlacklist: ["cfi-marker", "mo-cfi-highlight", "resize-sensor", "resize-sensor-expand", "resize-sensor-shrink", "resize-sensor-inner"],
            elementBlacklist: [],
            idBlacklist: ["MathJax_Message", "MathJax_SVG_Hidden"]
        });

@danielweck
Copy link
Member

https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L63

    var _cfiClassBlacklist = ["cfi-marker", "mo-cfi-highlight", "resize-sensor", "resize-sensor-expand", "resize-sensor-shrink", "resize-sensor-inner"];
    var _cfiElementBlacklist = [];
    var _cfiIdBlacklist = ["MathJax_Message", "MathJax_SVG_Hidden"];

@danielweck
Copy link
Member

Related issue (Feb. 2015):
readium/readium-cfi-js#30

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

2 participants