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

Styles in separate files are ignored #121

Open
wheelercj opened this issue Sep 13, 2024 · 2 comments
Open

Styles in separate files are ignored #121

wheelercj opened this issue Sep 13, 2024 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@wheelercj
Copy link
Collaborator

wheelercj commented Sep 13, 2024

When Stardown copies part of a page, it sometimes needs to look at the page's style for certain things such as whether an element is hidden (display: none, visibility: hidden, etc.) in Stardown's removeHiddenElements function. However, it appears to be unable to see styles defined in CSS files separate from the HTML.

To Reproduce
Steps to reproduce the behavior:

  1. Go to ⦋2306.03872⦌ Deductive Verification of Chain-of-Thought Reasoning
  2. Select the page's title, "Deductive Verification of Chain-of-Thought Reasoning"
  3. Copy with Stardown
  4. Paste
  5. See that the output includes "Title:Deductive Verification of Chain-of-Thought Reasoning"

Expected behavior
Title: should not be in the output.

Environment

  • OS: Windows
  • Browser: Firefox
  • Device: desktop

Additional context
The element containing Title: has style display: none defined in CSS files separate from the HTML. Stardown now uses getComputedStyle in its removeHiddenElements function to get element styles, but still fails to find the display: none.

Stardown's getSelectionFragment function converts a Selection object into a DocumentFragment object. Printing this object to the console and inspecting it reveals display: "". I'm not 100% sure if this means the DocumentFragment does not have access to styles that are not inline. I tried changing Stardown's selection-modifying code to include in each selection all of the selection's ancestor elements that include styles, but it had no effect so I removed the code without committing it.

Since the removeHiddenElements function calls getComputedStyle with the live document object, it seems like it should work even if the process of getting a Selection object and converting it to a DocumentFragment object somehow separates the HTML from the CSS, but it appears to not work.

@wheelercj wheelercj added the bug Something isn't working label Sep 13, 2024
@wheelercj
Copy link
Collaborator Author

wheelercj commented Oct 6, 2024

CSSStyleDeclaration - Web APIs | MDN

The documentation and online discussions make it sound like the following code should work, but it always returns an empty string: document.defaultView.getComputedStyle(node).getPropertyValue('display') (where node is from a DocumentFragment created from the current document). However, document.styleSheets[27].cssRules[2].style.display gives the string "none". Checking each CSS rule in each style sheet for each class in each element in the DocumentFragment seems extremely computationally inefficient though.

Here's some of the code I tried:

/** @type {CSSStyleDeclaration} */
const style = doc.defaultView.getComputedStyle(currentNode);
console.debug('---------------------------------------');
console.debug(`nodeName: ${currentNode.nodeName}`);
console.debug(`textContent: ${currentNode.textContent}`);
console.debug(`style: ${style}`);
console.debug(`style.getPropertyValue('display'): ${style.getPropertyValue('display')}`);
console.debug(`style.getPropertyValue('visibility'): ${style.getPropertyValue('visibility')}`);

Output from copying the page's title:

---------------------------------------
nodeName: H1
textContent: Title:Deductive Verification of Chain-of-Thought Reasoning
style: [object CSS2Properties]
style.getPropertyValue('display'):
style.getPropertyValue('visibility'):
---------------------------------------
nodeName: SPAN
textContent: Title:
style: [object CSS2Properties]
style.getPropertyValue('display'):
style.getPropertyValue('visibility'):

currentNode in the example above is a Node in a DocumentFragment that was created like this:

/** @type {DocumentFragment} */
let frag = document.createDocumentFragment();

/** @type {Range} */
const startRange = getStartRange(selection);
frag.appendChild(startRange.cloneContents());

selection above is a Selection object.

@wheelercj wheelercj added the help wanted Extra attention is needed label Oct 6, 2024
@wheelercj
Copy link
Collaborator Author

wheelercj commented Oct 6, 2024

Readability.js also does not remove the hidden elements on ⦋2306.03872⦌ Deductive Verification of Chain-of-Thought Reasoning. (The function isProbablyReaderable returns false for the page, but I temporarily commented-out that code so that Readability.js would run anyways.)

Here's the method Readability.js uses for finding hidden elements:

    _isProbablyVisible(node) {
        // Have to null-check node.style and node.className.includes to deal with SVG and MathML nodes.
        return (
            (!node.style || node.style.display != "none") &&
            (!node.style || node.style.visibility != "hidden") &&
            !node.hasAttribute("hidden") &&
            //check for "fallback-image" so that wikimedia math images are displayed
            (!node.hasAttribute("aria-hidden") ||
                node.getAttribute("aria-hidden") != "true" ||
                (node.className &&
                    node.className.includes &&
                    node.className.includes("fallback-image")))
        );
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant