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

yandex.ru search engine is still being added #16

Closed
shadowlmd opened this issue Jul 14, 2018 · 15 comments
Closed

yandex.ru search engine is still being added #16

shadowlmd opened this issue Jul 14, 2018 · 15 comments
Labels
site-specific Issues related to particular Web sites or pages

Comments

@shadowlmd
Copy link

Hello,

I'm using Chromium in Ubuntu, current version is 66.0.3359.181. Extension version 0.0.4 allows yandex.ru search engine to be added automatically after submitting making any search.

@cxw42 cxw42 added the site-specific Issues related to particular Web sites or pages label Jul 14, 2018
@cxw42
Copy link
Collaborator

cxw42 commented Jul 14, 2018

Thanks for the report! We'll look into it.

@cxw42 cxw42 changed the title Some search engines are still being added yandex.ru search engine is still being added Aug 7, 2018
@cxw42
Copy link
Collaborator

cxw42 commented Aug 7, 2018

Yandex's search field already has a name - maybe that makes a difference?

<input class="input__control input__input" tabindex="2" autocomplete="off" autocorrect="off" autocapitalize="off" spellcheck="false" aria-autocomplete="list" aria-label="Запрос" id="text" maxlength="400" autofocus="" name="text">

@Procyon-b
Copy link
Contributor

Procyon-b commented Aug 8, 2018

I have tried it too.
I think I know what is happening.
This page has 2 input fields. One normal with a name, the other hidden by default and without a name. It is used by the page to display "autocomplete" suggestion to the user. Since it has no name, like our textarea in its final form, it is not sent with the request.
I have made a modification in the code to ignore input fields with no name when it tests the form's structure (line 39 "if(elem.querySelectorAll...").
I don't know the syntax of -webkit-any() but it seems to work. Can you check if it's actually doing what I want: ignore unnamed form elements.

content.zip

[edit] by the way, here is a useful code to display the list of all forms and form elements of a page.
Paste this code in the javascript console of the page, and hit enter:

for (var i=0; i<document.forms.length; i++) for (var j=0; j<document.forms[i].length; j++) console.info({ i,j, html:document.forms[i][j].outerHTML, f:document.forms[i][j] } )

@Procyon-b
Copy link
Contributor

It is definitively not correct.
It might break on other sites.
The syntax as I have used it is not doing exactly what I want

@cxw42
Copy link
Collaborator

cxw42 commented Aug 8, 2018

OK- I will wait until I hear more before trying it myself :) . Welcome back! \o/

@Procyon-b
Copy link
Contributor

Procyon-b commented Aug 8, 2018

This version should work.
Going back to CSS basics, and here is my take on it.
Modifications: some spellchecking in my comments. Comment out a line of debug output (line 24, too many info on aol.com, and duplicate of "unspoiled" array), removed output of unnecessary text and empty array (lines 69-70)

Here is the fix: (line 39)
old
':scope input:-webkit-any([type="text" i],[type="search" i],:not([type])):not([readonly])'
new
':scope input:-webkit-any([type="text" i],[type="search" i],:not([type])):not([readonly])[name]'

This means that we only take named input/text into account.

content.zip

Edit: I'll check if name="" in html tags is caught by the CSS selector. It shouldn't.

@Procyon-b
Copy link
Contributor

Procyon-b commented Aug 8, 2018

I'll have to refine it to consider name="" as no name at all.
But it should work correctly except for some very specific cases.

cxw42 pushed a commit to cxw42/chrome-dont-add-custom-search-engines that referenced this issue Aug 8, 2018
@cxw42
Copy link
Collaborator

cxw42 commented Aug 8, 2018

Looks OK to me on yandex, uk.webuy, and files.scene.org :D . I did some cleanup - would you please work off the latest on #17 if you do refine it? Commit 6164c20, and attached here. Thanks!

@Procyon-b
Copy link
Contributor

Procyon-b commented Aug 8, 2018

Something is wrong with the cleanup.
I had to look for a couple of minutes before noticing that at line 47, the "===" breaks the code. Apparently when a variable is passed as is to "forEach()" (see line 68) it is transformed as an object when accessed through "this". If we want to use "===" we need to pass an object {now} instead of now and test on this.now ===

Edit;
PS: If you reinstate the debug line that I have commented out console.info({t:this, Found: elem}); (insert at line 24 of your code) you'll see how "this" ("t") is structured.

@cxw42
Copy link
Collaborator

cxw42 commented Aug 9, 2018 via email

@Procyon-b
Copy link
Contributor

I have a fully functional version here.
Updated CSS selector, and using === with this.now

content.zip

@Procyon-b
Copy link
Contributor

One last modification that we can do is increasing the timer delay. Push it up to 2 or 4 seconds, maybe.
This computer is a little bit slower and has less memory than the previous ones, so I ran into a "windows is swaping" event where chrome (I really had a large number of opened tabs) was taking a little bit longer than usual to reload uk.webuy.com. And this resulted in the timer firing too early and breaking the header of the page. It happened only once, and I haven't been able to reproduce it.

@cxw42
Copy link
Collaborator

cxw42 commented Aug 9, 2018

Hmm... That's a tough one. Would you open a separate issue, please? We might need to make that one a user option, or request tabs permission and count tabs.

@Procyon-b
Copy link
Contributor

Procyon-b commented Aug 9, 2018

No need to bother too much, 1.5 sec was chosen arbitrarily. We can select another value. It won't break anything at all.

cxw42 pushed a commit to cxw42/chrome-dont-add-custom-search-engines that referenced this issue Aug 9, 2018
Updated CSS selector, and using === with this.now in spoilFormGet
@cxw42
Copy link
Collaborator

cxw42 commented Sep 24, 2018

Should be fixed in #17 - closing. If you experience further problems, please feel free to comment back! Note that #17 hasn't been deployed yet, so you won't see the updates until @gregsadetsky rolls out v0.0.5.

@cxw42 cxw42 closed this as completed Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-specific Issues related to particular Web sites or pages
Projects
None yet
Development

No branches or pull requests

3 participants