Skip to content

Feature/fix pseudo elements #642

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

Closed
wants to merge 4 commits into from
Closed

Conversation

diba1013
Copy link

Before submission, please check that ...

  • the code follows the Clean Code guidelines.
  • the necessary tests are either created or updated.
  • all status checks (GitHub Actions, SonarCloud, etc.) pass.
  • your commit history is cleaned up.
  • you updated the changelog. (This is not released yet, so not necessary)
  • you updated necessary documentation within retest/docs. (see above)

Description

TL;DR This should fix ghost pseudo elements that are added, despite not rendered by the browser.

As already described by #625, ghost pseudo elements cause the alignment algorithm to get confused, which produces "a" has been inserted and "a" has been deleted difference. This PR should address the most pressing problems and catch the currently known cases (mostly as a workaround).

Bug

I have added a test which shows only one occurrence of the bug. It appears consistently, if a button (or link) is pressed and a check is generated at this exact moment or a few moments later—I have yet to figure out, when Selenium "completes" a click. Because buttons make use of a pseudo class (:pressed), it will change some properties of the button (e.g. outline) to show the button has been pressed.

This has the current algorithm detect changes in the parents' and the pseudo elements' style and generate a pseudo element which then confuses the alignment algorithm.

Problem

  1. Inherited attributes

    In general, attributes of a parent is inherited by the child element. Since pseudo elements (especially ::before and ::after) are considered direct children of the parent attributes.

    Strangely enough, pseudo class changes are not propagated to the getComputedStyle(parent, "$pseudo") which returns the old CSS attributes (i.e. not pressed). I have tried to add the parents' states to the query, but the browser refuses to give the current state. I can only assume that this is by design, that pseudo classes (e.g. :hover) are not passed down to the child to allow them to respond independently to them.

  2. Rendering of pseudo elements

    While most pseudo elements are always present (although rarely used), the ::before and ::after pseudo elements are not always added to the render tree. That is because the specification describes they should only be added if their content attribute is not "none". This is currently ignored by the implementation, which only looks at the inherited attributes.

  3. CSS Attributes

    According to the specification, not all attributes are allowed on pseudo elements. However, we query attributes on the pseudo elements that will never affect the element (e.g. outline) and thus return a false value.

Implementation

  1. The most used pseudo elements (::before, ::after) are easy to exclude, following the specification.
  2. The other pseudo elements are not trivial, because they can only be identified by their CSS Attributes. However, since this is much work, I have only included a workaround for the current problem (i.e. hack) and will discuss a more concrete solution in a separate issue (issue link pending).

State of this PR

  1. Revert the first commits, as they are only included to have the test run. This should partly be addressed by Disable rehub upload #638.
  2. There are tests failing, some of them make sense, but others now produce the dreaded inserted-deleted difference. I did not do a great amount of investigation (mostly because of the now lacking accepting possibilities), but wanted to get this PR on the way to gather opinions. @githubert would it be too much to ask to glance over them, since you have already battled those in the previous PRs (we could do that together, too)?
  3. This requires more testing? What tests are covered by the existing PseudoElementIT?

Additional Context

  1. I will discuss a proper solution as part of a separate issue (issue link pending).
  2. This is related to Fix tests that broke after introducing pseudo elements #624, although the original test have been fixed?
  3. Please refer to the pseudo element specification.
  4. There is a draft to make pseudo elements more accessible to JavaScript. However, it will take some time until the major browsers will support this.

@diba1013 diba1013 added the bug Something isn't working label Aug 26, 2020
@diba1013 diba1013 self-assigned this Aug 26, 2020
@cla-bot cla-bot bot added the cla-signed label Aug 26, 2020
@diba1013
Copy link
Author

diba1013 commented Aug 29, 2020

I tried to update the tests locally, but due to different OS (linux vs windows), the tests are not passing locally. I think that we have to upload the reports to GitHub Actions, like @oezelenes looked into some time ago.

Furthermore, there is still this trouble within recheck(.cli) when applying inserted-deleted elements, which seem to not to remove the element properly and thus causes duplication with bestMatch and previousMatch have a match of 100%? (as described by retest/recheck#598)

@cla-bot
Copy link

cla-bot bot commented Jun 4, 2021

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla-signed label Jun 4, 2021
@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from 675764b to 69d9e02 Compare January 18, 2022 15:09
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from 69d9e02 to 0a4486c Compare January 18, 2022 15:14
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from 0a4486c to a2390cc Compare January 18, 2022 15:19
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from a2390cc to d3eb089 Compare January 18, 2022 15:24
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from d3eb089 to ab65538 Compare January 19, 2022 00:27
@cla-bot
Copy link

cla-bot bot commented Jan 19, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

diba1013 added 4 commits January 19, 2022 00:31
as the w3c specifies that these pseudo elements should only be added, if `content != "none"`
since those do not have a hard criteria but instead only a subset of possible css attributes they accept. However, since we currently extract all css attributes, there are those that do not apply (e.g outline). Furthermore, css states (e.g. :pressed) is not propagated downwards, so the outline of the pseudo element cannot change and therefore will always be different to the parent.
@rebazer rebazer force-pushed the feature/fix-pseudo-elements branch from ab65538 to 4328720 Compare January 19, 2022 00:31
@cla-bot
Copy link

cla-bot bot commented Jan 19, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@diba1013 diba1013 closed this Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants