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

[FIX] web: no chrome autocompete on many2one widget #30439

Closed
wants to merge 1 commit into from

Conversation

nle-odoo
Copy link
Contributor

@nle-odoo nle-odoo commented Jan 22, 2019

For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have autocomplete="off" to prevent browser completion.

But chromium has an history of breaking autocomplete="off", see:

It seems that recently (chromium 71 at least), the heuristic to ignore
autocomplete="off" has become more aggressive and for example on
Many2one with placeholder "Country", chromium will display browser
autocomplete:

  • hidding the many2one autocomplete
  • having field empty when visually filled if the autocomplete result is
    selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

@nle-odoo nle-odoo added the OE the report is linked to a support ticket (opw-...) label Jan 22, 2019
@nle-odoo
Copy link
Contributor Author

I worked on a first solution: a6e71b7f4e53 that removed/added back the placeholder when the many2one autocomplete was opened/closed.

But there was side effects (the placeholder is removed at first click, the browser autocomplete is shown the first time until something is written in the input, ...).

The new solution is more simple and seem to work well but there is downside such as: someone selecting input[placeholder="Country"] in CSS or javascript, and I don't know how screen reader would react.

Copy link
Contributor

@len-foss len-foss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you nle and thanks to our great overlords at Google. May our prayers reach them.

@nle-odoo
Copy link
Contributor Author

nle-odoo commented Jan 22, 2019

@KangOl here the solution is rather hackish (the fix should be merged in 10.0)

do you think this is alright to do this?

this may be solved at one point by chromium and this could be reverted in Odoo but currently the issue breaks functionality in google chrome of some many2one widgets (Country and State for example)

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Jan 22, 2019
@KangOl
Copy link
Contributor

KangOl commented Jan 22, 2019

I propose to forbid using chrome and ask people to install firefox.

@KangOl
Copy link
Contributor

KangOl commented Jan 22, 2019

Can you please link to this PR in code comment?

For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have `autocomplete="off"` to prevent browser completion.

But chromium has an history of breaking `autocomplete="off"`, see:

- https://caniuse.com/#search=autocomplete
- https://crbug.com/468153
- https://crbug.com/587466
- https://crbug.com/914451

It seems that recently (chromium 71 at least), the heuristic to ignore
`autocomplete="off"` has become more aggressive and for example on
Many2one with placeholder "Country", chromium will display browser
autocomplete:

- hidding the many2one autocomplete
- having field empty when visually filled if the autocomplete result is
  selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

opw-1930588
closes odoo#30439
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Jan 22, 2019
@nle-odoo
Copy link
Contributor Author

I added , see #30439 and did PR for 10.0 with same code #30449

robodoo pushed a commit that referenced this pull request Jan 22, 2019
For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have `autocomplete="off"` to prevent browser completion.

But chromium has an history of breaking `autocomplete="off"`, see:

- https://caniuse.com/#search=autocomplete
- https://crbug.com/468153
- https://crbug.com/587466
- https://crbug.com/914451

It seems that recently (chromium 71 at least), the heuristic to ignore
`autocomplete="off"` has become more aggressive and for example on
Many2one with placeholder "Country", chromium will display browser
autocomplete:

- hidding the many2one autocomplete
- having field empty when visually filled if the autocomplete result is
  selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

10.0 version of #30439
opw-1930588
closes #30449
@Elkasitu
Copy link
Contributor

botnet destroyed with a single unicode character 👍

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Jan 23, 2019
For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have `autocomplete="off"` to prevent browser completion.

But chromium has an history of breaking `autocomplete="off"`, see:

- https://caniuse.com/#search=autocomplete
- https://crbug.com/468153
- https://crbug.com/587466
- https://crbug.com/914451
- https://crbug.com/923895

It seems that since chromium 71, the heuristic to ignore
`autocomplete="off"` has become more aggressive and for example if there
is at least 3 fields like an address in a page, chromium will ignore
`autocomplete="off"` for the fields like an address.

So for example the eidting the many2One field with placeholder "Country"
in a contact page now has a browser autocomplete menu that is:

- hidding the many2one autocomplete
- going to save empty country it appeared visually filled if the
  autocomplete result was selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

10.0 version of odoo#30439
opw-1930588
closes odoo#30439
closes odoo#30449
robodoo pushed a commit that referenced this pull request Jan 23, 2019
For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have `autocomplete="off"` to prevent browser completion.

But chromium has an history of breaking `autocomplete="off"`, see:

- https://caniuse.com/#search=autocomplete
- https://crbug.com/468153
- https://crbug.com/587466
- https://crbug.com/914451
- https://crbug.com/923895

It seems that since chromium 71, the heuristic to ignore
`autocomplete="off"` has become more aggressive and for example if there
is at least 3 fields like an address in a page, chromium will ignore
`autocomplete="off"` for the fields like an address.

So for example the eidting the many2One field with placeholder "Country"
in a contact page now has a browser autocomplete menu that is:

- hidding the many2one autocomplete
- going to save empty country it appeared visually filled if the
  autocomplete result was selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

10.0 version of #30439
opw-1930588
closes #30439
closes #30449
@nle-odoo
Copy link
Contributor Author

I searched a bit more, this is an issue only since chrome 71.

If we have 3 fields that chrome see as "address-like", eg (jsfiddle):

<input placeholder="City" autocomplete="off"//>
<input placeholder="State" autocomplete="off"/>
<input placeholder="Country" autocomplete="off"/>

chrome totally ignores autocomplete="off".

So the heuristic seems to be: if there is 3 or more address-like field on a page, ignore autocomplete="off" for all address-like field on that page.

I added a report for this exact issue in this existing bug report for chromium 71: https://crbug.com/923895#c5

robodoo pushed a commit that referenced this pull request Jan 23, 2019
For editing many2one fields (and partially many2many) most widget show
an autocompleting list of targeted records.

They thus have `autocomplete="off"` to prevent browser completion.

But chromium has an history of breaking `autocomplete="off"`, see:

- https://caniuse.com/#search=autocomplete
- https://crbug.com/468153
- https://crbug.com/587466
- https://crbug.com/914451
- https://crbug.com/923895

It seems that since chromium 71, the heuristic to ignore
`autocomplete="off"` has become more aggressive and for example if there
is at least 3 fields like an address in a page, chromium will ignore
`autocomplete="off"` for the fields like an address.

So for example the eidting the many2One field with placeholder "Country"
in a contact page now has a browser autocomplete menu that is:

- hidding the many2one autocomplete
- going to save empty country it appeared visually filled if the
  autocomplete result was selected.

With this changeset, the placeholder in the many2one instance is
interspersed with U+FEFF charcters (ZERO WIDTH NO-BREAK SPACE) so the
browser does enable the autocomplete feature by force.

This should thus remove the issue (until it is fixed by chromium) in the
case of field named "Country" or matching other regexes in this file:

https://github.com/chromium/chromium/blob/cdb1b2073f12/components/autofill/core/common/autofill_regex_constants.cc

U+FEFF has been chosen instead of more recommended characters because
other have been shown erroneous for printing in some windows
configuration (see cb2a3af).

10.0 version of #30439
opw-1930588
closes #30439
closes #30449
@nle-odoo
Copy link
Contributor Author

nle-odoo commented Jan 23, 2019

merged in 10.0 with 90c1af1151c

@nle-odoo nle-odoo closed this Jan 23, 2019
@nle-odoo nle-odoo deleted the 12.0-web-opw-1930588-nle branch January 23, 2019 14:19
@JulienCochuyt
Copy link

The new solution is more simple and seem to work well but there is downside such as: someone selecting input[placeholder="Country"] in CSS or javascript, and I don't know how screen reader would react.

This approach definitely breaks screen readers ability to properly report the placeholder.
Tested with JAWS 18.0 and NVDA 2018.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants