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.

Sign up for GitHub

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

Jump to bottom

Improve location-helpers to work across different scenarios #63

Closed
3 tasks
molant opened this issue Mar 30, 2017 · 3 comments
Closed
3 tasks

Improve location-helpers to work across different scenarios #63

molant opened this issue Mar 30, 2017 · 3 comments
Labels
area:helpers type:technical-debt
Projects
v1

Comments

@molant
Copy link
Member

molant commented Mar 30, 2017 •
edited

We need to:

  • find a way to calculate the offset automatically (if needed)
    • update the tests so we no longer have <!doctype html><html><head> in the same line
  • make sure we get the same results for all collectors
@molant molant mentioned this issue Mar 31, 2017
Closed
alrra pushed a commit that referenced this issue Apr 3, 2017
@molant @alrra
Fix issue with location-helpers
edd328f 
* Fix issue in `location-helpers` related to calculating the position
  of an error when `content` is passed to `findProblemLocation` and the
  problem is in the first line of the element's HTML.
* Update tests that had the issue described above.
* Update the HTML of the tests to put `doctype`, `html`, and `head` in
  the same line until #63 is fixed entirely.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Ref #63
@molant molant mentioned this issue Apr 3, 2017
Closed
@molant molant added this to TODO in v1 Apr 3, 2017
@molant molant moved this from TODO to Commited in v1 Apr 4, 2017
@molant
Copy link
Member Author

molant commented Apr 10, 2017

We can use serializeDocument from JSDOM to figure the right offset

@molant molant moved this from Committed to In Progress in v1 Apr 12, 2017
@molant molant moved this from In Progress to Committed in v1 Apr 12, 2017
@molant molant moved this from Committed to TODO in v1 Apr 17, 2017
@molant molant moved this from TODO to In Progress in v1 Apr 19, 2017
@molant molant self-assigned this Apr 19, 2017
@molant molant mentioned this issue Apr 19, 2017
Closed
@molant
Copy link
Member Author

molant commented Apr 19, 2017

The current differences we have are:

  • JSDOM doesn't return <!doctype html> with outerHTML
  • JSDOM and CDP collapse doctype, html and head in one single line

Because HTML allows to not have html, head and body tags, and some times a website's markup can be really scary, I'm leaning towards having a aprox location for the body and a location of the issue in the elements outerHTML.

The rules will use findInElement and rule-context will do the magic to find it in the document.

We should still try to find the right location in the document for a given element, but switching to this model will make testing rules a lot easier (at least with what we have right now).

@molant
Copy link
Member Author

molant commented Apr 26, 2017

We could use parse5 to get the location of a dom. This is what jsdom v10 uses (we should update our code to use it), not sure how we will do with cdp but we should definitely look into it.

alrra pushed a commit that referenced this issue Apr 27, 2017
@molant @alrra
Change how location works
b008689 
Per [this comment](#63 (comment))
we are changing to just report the issue relative to the element and
(in the future) let `rule-context` figure out the rest.

This unblocks us to continue testing `collector`s and make sure they
are consistent at least in that aspect.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Work on #63
Work on #113
alrra pushed a commit that referenced this issue Apr 27, 2017
@molant @alrra
Change how location works
658a1c4 
Per [this comment](#63 (comment))
we are changing to just report the issue relative to the element and
(in the future) let `rule-context` figure out the rest.

This unblocks us to continue testing `collector`s and make sure they
are consistent at least in that aspect.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Work on #63
Work on #113
@molant molant moved this from In Progress to Committed in v1 May 8, 2017
@molant molant moved this from Committed to Backlog in v1 May 10, 2017
@sarvaje sarvaje self-assigned this May 22, 2017
@sarvaje sarvaje moved this from Backlog to In Progress in v1 May 22, 2017
@sarvaje sarvaje mentioned this issue May 24, 2017
Merged
@sarvaje sarvaje moved this from In Progress to In review in v1 May 25, 2017
molant referenced this issue in sarvaje/Sonar-old May 26, 2017
@sarvaje
Getting the position approximate of an element in the html
d318688 
Due to JSDOM and CDP have a similar behavior with the property outerHTML. We are going to use it and we should let the users know that the posision is approximate not exactly. The "problem" is that not only the tags html and header are moved to the first line but there are other elements that also are moved when we use the outerHTML, but this property is the only way (AFAIK) to get the current HTML in an SPA.
Fix how we wait to all the request finish in CDP

Fix #63
molant pushed a commit that referenced this issue May 26, 2017
@sarvaje @molant
Getting the position approximate of an element in the html
01d828f 
Due to JSDOM and CDP have a similar behavior with the property outerHTML. We are going to use it and we should let the users know that the posision is approximate not exactly. The "problem" is that not only the tags html and header are moved to the first line but there are other elements that also are moved when we use the outerHTML, but this property is the only way (AFAIK) to get the current HTML in an SPA.
Fix how we wait to all the request finish in CDP

Fix #63
@molant molant moved this from In review to Done in v1 May 26, 2017
@zelopy zelopy mentioned this issue Nov 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:helpers type:technical-debt
Projects
No open projects
v1
Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@molant @sarvaje