minbrowser / min

A fast, minimal browser that protects your privacy
https://minbrowser.org/
Apache License 2.0
7.91k stars 705 forks source link

key icons appearing randomly #1331

Open shalva97 opened 3 years ago

shalva97 commented 3 years ago

Expected Behavior

I should only see key icons for login pages

Actual Behavior

on Github:

image

on my locally hosted qbittorrent:

image

on some random russian site, which is a request form for support

image

To Reproduce

PalmerAL commented 3 years ago

For the second two, can you right click > inspect element, and post the HTML for the input elements?

shalva97 commented 3 years ago

from qbittorrent

<table style="margin: auto;">
                <tbody><tr>
                    <td>
                        <label for="autoTMM">Torrent Management Mode:</label>
                    </td>
                    <td>
                        <select id="autoTMM" name="autoTMM" onchange="qBittorrent.Download.changeTMM(this)">
                            <option selected="" value="false">Manual</option>
                            <option value="true">Automatic</option>
                        </select>
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="savepath">Save files to location:</label>
                    </td>
                    <td>
                        <input type="text" id="savepath" name="savepath" style="width: 16em;">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="rename">Rename torrent</label>
                    </td>
                    <td>
                        <input type="text" id="rename" name="rename" style="width: 16em;">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="category">Category:</label>
                    </td>
                    <td>
                        <div class="select-watched-folder-editable">
                            <select id="categorySelect" onchange="qBittorrent.Download.changeCategorySelect(this)">
                                <option selected="" value="\other"></option>
                            </select>
                            <input name="category" type="text" value="">
                        </div>
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="startTorrent">Start torrent</label>
                    </td>
                    <td>
                        <input type="hidden" id="startTorrentHidden" name="paused">
                        <input type="checkbox" id="startTorrent">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="skip_checking">Skip hash check</label>
                    </td>
                    <td>
                        <input type="checkbox" id="skip_checking" name="skip_checking" value="true">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="rootFolder">Create subfolder</label>
                    </td>
                    <td>
                        <input type="hidden" id="rootFolderHidden" name="root_folder">
                        <input type="checkbox" id="rootFolder">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="sequentialDownload">Download in sequential order</label>
                    </td>
                    <td>
                        <input type="checkbox" id="sequentialDownload" name="sequentialDownload" value="true">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="firstLastPiecePrio">Download first and last pieces first</label>
                    </td>
                    <td>
                        <input type="checkbox" id="firstLastPiecePrio" name="firstLastPiecePrio" value="true">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="dlLimitText">Limit download rate</label>
                    </td>
                    <td>
                        <input type="hidden" id="dlLimitHidden" name="dlLimit">
                        <input type="text" id="dlLimitText" style="width: 16em;" placeholder="KiB/s">
                    </td>
                </tr>
                <tr>
                    <td>
                        <label for="upLimitText">Limit upload rate</label>
                    </td>
                    <td>
                        <input type="hidden" id="upLimitHidden" name="upLimit">
                        <input type="text" id="upLimitText" style="width: 16em;" placeholder="KiB/s">
                    </td>
                </tr>
            </tbody></table>

image

shalva97 commented 3 years ago

https://www.twirpx.com/

<div class="form">
      <div class="field required">
        <textarea id="input_topic" class="text" rows="2" placeholder="Тема"></textarea>
      </div>
      <div class="field">
        <select class="select" id="input_type">
            <option>Тип работы</option>
            <optgroup label="Дипломные">
                <option value="Диплом МВА">Диплом МВА</option>
                <option value="Магистерский диплом">Магистерский диплом</option>
                <option value="Дипломная работа (бакалавр/специалист)">Дипломная работа (бакалавр/специалист)</option>
                <option value="Дипломная работа (колледж/техникум)">Дипломная работа (колледж/техникум)</option>
                <option value="Аттестационная работа (ВАР/ВКР)">Аттестационная работа (ВАР/ВКР)</option>
                <option value="Часть дипломной работы">Часть дипломной работы</option>
                <option value="Сопроводительные материалы к диплому">Сопроводительные материалы к диплому</option>
            </optgroup>
            <optgroup label="Курсовые и отчеты">
                <option value="Курсовая с практикой">Курсовая с практикой</option>
                <option value="Курсовая теория">Курсовая теория</option>
                <option value="Отчёт по практике">Отчёт по практике</option>
            </optgroup>
            <optgroup label="Рефераты и задачи">
                <option value="Реферат">Реферат</option>
                <option value="Реферат для аспирантуры">Реферат для аспирантуры</option>
                <option value="Контрольная работа">Контрольная работа</option>
                <option value="Задачи">Задачи</option>
                <option value="Кейсы">Кейсы</option>
                <option value="Статья">Статья</option>
                <option value="Тест">Тест</option>
                <option value="Чертежи">Чертежи</option>
                <option value="Эссе">Эссе</option>
            </optgroup>
            <optgroup label="Другие">
                <option value="Бизнес-план">Бизнес-план</option>
                <option value="Вопросы к экзамену">Вопросы к экзамену</option>
                <option value="Лабораторная работа, РГР">Лабораторная работа, РГР</option>
                <option value="Он-лайн помощь">Он-лайн помощь</option>
                <option value="Поиск информации">Поиск информации</option>
                <option value="Презентация в PowerPoint">Презентация в PowerPoint</option>
                <option value="Другое">Другое</option>
            </optgroup>
        </select>
      </div>
      <div class="field required">
        <input type="text" class="text" id="input_subject" placeholder="Предмет">
      </div>      
      <div class="field">
        <input type="text" class="text hasDatepicker" id="input_date" placeholder="Когда нужна работа?">
      </div>
      <div class="field">
        <textarea id="input_details" class="text" rows="5" placeholder="Требования к работе"></textarea>
      </div>
      <div class="field">
        <input type="text" class="text" id="input_phone" placeholder="Телефон (ответ придет СМС)">
      </div>
      <div class="field required">
        <input type="text" class="text" id="input_email" placeholder="Ваш e-mail">
      </div>
      <div class="field">
        <input type="checkbox" class="checkbox" id="input_personal_data" checked="checked">
        <label for="input_personal_data">Даю согласие на <a href="https://assist.multiwork.org/personal-data.pdf" target="_blank">обработку персональных данных</a></label>
      </div>
      <div class="button">
        <input type="button" class="button" id="button_submit" value="Оценить бесплатно" default_value="Оценить бесплатно">
        <p id="p_message" class="message" style="display: none"></p>
      </div>
    </div>

image

PalmerAL commented 3 years ago

Cases 2 and 3 are fixed in aad945992d155c6cf9919de1e50bd3a0fe51c186. The Github case is harder because they also have something that looks like a password field in the page; I think for now it's better to leave it as is - if we make the criteria for when to show the icon too strict, autofill may not work on some login pages, which would be worse.

benstigsen commented 3 years ago

Could it be because the GitHub author search field is:

<input class="SelectMenu-input form-control js-filterable-field" id="author-filter-field" type="text" placeholder="Filter users" aria-label="Filter users" autocomplete="off" spellcheck="false" autofocus="">

Where it contains id="author-filter-field"

In script js/preload/passwordFill.js I was able to remove the keyword icon by removing auth from the function

function getUsernameFields () {
  return getInputs(['user', 'name', 'mail', 'login', 'auth', 'identifier'], ['text', 'email'])
  //                                                  ^^^^
}

So I'm guessing it doesn't search for entire words, but only partial includes (line 87) https://github.com/minbrowser/min/blob/dcd0edc092dec4bb1b3bac688573444e53031abc/js/preload/passwordFill.js#L84-L112

benstigsen commented 3 years ago

Perhaps a way to fix this would be to split each attribute into a word (regex might be overkill), convert to lower, then check for entire instead of partial matches.

shalva97 commented 3 years ago

I found one more https://www.artstation.com/?sort_by=community

image

benstigsen commented 3 years ago

I think I have a solution to this. I'll do some more testing but so far it seems to fix it.

shalva97 commented 3 years ago

I can confirm all of them are fixed except https://www.artstation.com/?sort_by=community

shalva97 commented 3 years ago

I found another one with Min 1.17.3 Register on ufile.io then upload a file and choose Edit File. it will bring up this dialog with unnecessary key icons:

image

benstigsen commented 3 years ago

Yep, it's most likely because of id="display_name". The issue is known, and a solution for it has been found. I'll fix it as soon as I have the time available, thanks for the help.

benstigsen commented 3 years ago

I've had trouble fixing this because of my inexperience with JS and Regex 😅. I know the problem and solution, but implementing it has not been easy for me, even though it only requires changing 3 lines of code.

The problem:

Currently a key icon is shown where it shouldn't be. image This is what the author search field looks like:

<input class="SelectMenu-input form-control js-filterable-field" id="author-filter-field" type="text" placeholder="Filter users" aria-label="Filter users" autocomplete="off" spellcheck="false" autofocus="">

In the id of the field it can be seen that it's id="author-filter-field".

When looking at the way input fields are determined https://github.com/minbrowser/min/blob/9a7660dc88f53ef517de1844ce07e30de7761954/js/preload/passwordFill.js#L116 We can see that it checks if the username field containing "auth". This is where it goes wrong, because since the id is "author-filter-field" it technically does contain the string "auth".

The solution:

The following line: https://github.com/minbrowser/min/blob/9a7660dc88f53ef517de1844ce07e30de7761954/js/preload/passwordFill.js#L87 Should not be using .includes() but instead it should use a regex match. Something like auth((-\w+)+|$) should work, which makes things like these valid:

By changing .includes() to use regex, it should be possible to edit: https://github.com/minbrowser/min/blob/9a7660dc88f53ef517de1844ce07e30de7761954/js/preload/passwordFill.js#L116 and: https://github.com/minbrowser/min/blob/9a7660dc88f53ef517de1844ce07e30de7761954/js/preload/passwordFill.js#L121

To use regexes instead of something like basic strings.

PalmerAL commented 3 years ago

I think something like this should work:

input.id.split(/\W+/g).some(word => matches.includes(word.toLowerCase())

The split() would break "auth-login-field" into ["auth", "login", "field"], and then you could see if one of the words matches exactly.

(That probably needs to be adjusted a bit to fit into the existing code; I haven't looked at it yet).

If you don't want to work on this any more, that's totally fine too - you're not obligated to.

benstigsen commented 3 years ago

A fix for this has been made, it can be merged, but some things should probably be considered (as I've written in the PR).

For now this works. But perhaps it should use regex in the array of username names/IDs. So that instead of: [..., 'user', 'name', 'username', ...] it becomes: [..., /(user(name)?|name))/g, ...]

This would make it easier to create regexes that catch general use cases as well as just specific text, instead of adding to what might possibly become a very long array.

https://github.com/minbrowser/min/pull/1345

shalva97 commented 3 years ago

any updates on this?

PalmerAL commented 3 years ago

The field detection logic was improved recently in #1560. The other PR couldn't be merged because it broke detection on a bunch of sites (and Ben told me he doesn't have time to continue working on it any more).

I tested these again, and it looks like Github is fixed; I've fixed ufile in 825502320fe698dbaff14546b77458dd711fc042. Artstation is a different issue; they auto-focus the username input in their login form when the page loads, even though it's hidden offscreen, which causes the key icon to show up. (You can try this out yourself - if you let the page load, start typing something, and then click "sign in", what you typed will be in the username field). So we probably need a check for if the field is visible or not.

shalva97 commented 3 years ago

I guess that would be the last case... nice work so far