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

DOMContentLoaded/load timer; name="" #17

Merged
merged 5 commits into from Sep 24, 2018
Merged

Conversation

cxw42
Copy link
Collaborator

@cxw42 cxw42 commented Aug 7, 2018

This is the proposed fix for #14 from @Procyon-b - as zipped here. I am adding the PR since Procyon-b is not able to work on it at this time.

I have not yet tested this, so please try it out and let me know what you find! Please also check yandex per #16.

Assuming it works, it closes #14 and closes #15.

Before merging, we will need an additional commit to turn off DEBUG.

@cxw42
Copy link
Collaborator Author

cxw42 commented Aug 7, 2018

Works with uk.webuy.com, but doesn't spoil Yandex.

@Procyon-b
Copy link
Contributor

I'm back. Finally ! :)

I still have to finalize some configurations, and installation of utilities and I'm ready to go.

@gregsadetsky
Copy link
Owner

@Procyon-b glad to have you back! && Thank you for your work!

Updated CSS selector, and using === with this.now in spoilFormGet
@cxw42
Copy link
Collaborator Author

cxw42 commented Aug 9, 2018

Updated per discussion in #16, and looking good to me so far!

@Procyon-b
Copy link
Contributor

Question.
I was about to add a pull request for the last version of content.js. I already have a fork (used for my first PR). I see that the branch is 6 commits behind, and "compare" shows nothing. What should I do? Refork? Somehow resync?
Note: I'm using the web interface.

@cxw42
Copy link
Collaborator Author

cxw42 commented Sep 7, 2018

Forks unfortunately don't keep up to date with upstream repositories automatically :( . The easiest thing might just be to delete your current fork and then re-fork this repo!

I found an extension that claims to update your fork to match master, but I haven't tried it myself - https://chrome.google.com/webstore/detail/github-sync-fork/omjaffmdnnkgmbbjmdalehkjcaklleii/ . Other thoughts are at:

(but note that if you do any of the Web-based options, you need to rebase rather than merging.)

And the official support page, using the CLI: https://help.github.com/articles/syncing-a-fork/

@gregsadetsky have you had a chance to test this PR? I have been running this code and haven't noticed any issues, but I haven't had a chance to run through the whole list on the wiki and try them out.

@Procyon-b
Copy link
Contributor

re. methods to sync:
I'll refork, this is the easiest way for the moment. I asked before doing this in case there was a simpler method I didn't notice.
What do you do in GitHub desktop? If that's what you are using.

re. Testing.
I have tested on several sites mentionned here and there with the version before the latest little fix. I don't remember if I used a list.

@cxw42
Copy link
Collaborator Author

cxw42 commented Sep 7, 2018

@Procyon-b I use the command line - I have Cygwin installed and so do git pull upstream master / git push to update origin (my fork) from upstream) (this repo).

@Procyon-b
Copy link
Contributor

Procyon-b commented Sep 7, 2018

re: Testing

I remember now using that list to test. I have retried all these hosts and all seems ok.

Edit: bestbuy.com failed
Edit2: and I can't reproduce it

I think we will have to live with the fact that some sites sometimes will fail us.

Edit 3: I can reproduce when I fill in the field and submit within the first second of chrome displaying the page. Not a realistic situation.

@cxw42 cxw42 merged commit d0adc93 into gregsadetsky:master Sep 24, 2018
@cxw42 cxw42 deleted the pull-for-14 branch September 24, 2018 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The name of the added textarea element AOL.com still slips through
3 participants