Bug 191969

Summary:

REGRESSION (r238078): Do not draw caps lock indicator when Strong Password button is shown

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

Layout and Rendering

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, dino, ryanhaddad, simon.fraser, tsavell, webkit-bug-importer, wenson_hsieh, zalan

Priority:

Keywords:

InRadar, Regression

Version:

WebKit Local Build

Hardware:

iPhone / iPad

OS:

iOS 12

Bug Depends on:

190565

Bug Blocks:

Attachments:

Description	Flags
Patch and layout tests	none

Daniel Bates

Reported 2018-11-26 10:05:43 PST

Following r238078 we now support drawing the caps lock indicator in password fields on iOS. However it is not meaningful to show the caps lock indicator when the Strong Password button is visible because the password field is not editable. We should not paint the caps lock indicator when the Strong Password button is visible.

Attachments
Patch and layout tests (13.11 KB, patch) 2018-11-26 10:24 PST, Daniel Bates	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-11-26 10:06:14 PST

<rdar://problem/46247569>

Daniel Bates

Comment 2 2018-11-26 10:24:02 PST

Created attachment 355650 [details] Patch and layout tests

Daniel Bates

Comment 3 2018-11-26 12:42:26 PST

Comment on attachment 355650 [details] Patch and layout tests Clearing flags on attachment: 355650 Committed r238513: <https://trac.webkit.org/changeset/238513>

Daniel Bates

Comment 4 2018-11-26 12:42:28 PST

All reviewed patches have been landed. Closing bug.

Truitt Savell

Comment 5 2018-11-26 16:02:35 PST

The new test fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after-hiding-auto-fill-strong-password-button.html is failing constantly after being added. the diff shows the I beam is in the wrong place in the password field. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock-indicator-should-be-visible-when-after-hiding-auto-fill-strong-password-button.html Diff: https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/r238522%20(1020)/fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after-hiding-auto-fill-strong-password-button-diffs.html

Daniel Bates

Comment 6 2018-11-26 16:48:16 PST

(In reply to Truitt Savell from comment #5) > The new test > fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after- > hiding-auto-fill-strong-password-button.html is failing constantly after > being added. > > the diff shows the I beam is in the wrong place in the password field. > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock- > indicator-should-be-visible-when-after-hiding-auto-fill-strong-password- > button.html > > Diff: > https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/ > r238522%20(1020)/fast/forms/auto-fill-button/caps-lock-indicator-should-be- > visible-when-after-hiding-auto-fill-strong-password-button-diffs.html Skip the test for now. Will fix the test shortly.

Ryan Haddad

Comment 7 2018-11-26 17:03:17 PST

(In reply to Daniel Bates from comment #6) > (In reply to Truitt Savell from comment #5) > > The new test > > fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after- > > hiding-auto-fill-strong-password-button.html is failing constantly after > > being added. > > > > the diff shows the I beam is in the wrong place in the password field. > > > > History: > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock- > > indicator-should-be-visible-when-after-hiding-auto-fill-strong-password- > > button.html > > > > Diff: > > https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/ > > r238522%20(1020)/fast/forms/auto-fill-button/caps-lock-indicator-should-be- > > visible-when-after-hiding-auto-fill-strong-password-button-diffs.html > > Skip the test for now. Will fix the test shortly. We shouldn't skip brand new tests. I think we should roll out this change and re-land it with a working test.

Daniel Bates

Comment 8 2018-11-26 17:38:00 PST

(In reply to Ryan Haddad from comment #7) > (In reply to Daniel Bates from comment #6) > > (In reply to Truitt Savell from comment #5) > > > The new test > > > fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after- > > > hiding-auto-fill-strong-password-button.html is failing constantly after > > > being added. > > > > > > the diff shows the I beam is in the wrong place in the password field. > > > > > > History: > > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > > html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock- > > > indicator-should-be-visible-when-after-hiding-auto-fill-strong-password- > > > button.html > > > > > > Diff: > > > https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/ > > > r238522%20(1020)/fast/forms/auto-fill-button/caps-lock-indicator-should-be- > > > visible-when-after-hiding-auto-fill-strong-password-button-diffs.html > > > > Skip the test for now. Will fix the test shortly. > We shouldn't skip brand new tests. I think we should roll out this change > and re-land it with a working test. Feel free to roll it out.

Ryan Haddad

Comment 9 2018-11-26 18:00:06 PST

Rolled out in https://trac.webkit.org/changeset/238540/webkit

Daniel Bates

Comment 10 2018-11-26 20:17:19 PST

Committed r238545: <https://trac.webkit.org/changeset/238545>

Truitt Savell

Comment 11 2018-11-27 12:03:55 PST

The new test fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-after-hiding-auto-fill-strong-password-button.html added in https://trac.webkit.org/changeset/238545/webkit is timing out at around 300 seconds on Mojave WK2. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock-indicator-should-be-visible-after-hiding-auto-fill-strong-password-button.html

Daniel Bates

Comment 12 2018-11-27 13:21:10 PST

(In reply to Truitt Savell from comment #11) > The new test > fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-after- > hiding-auto-fill-strong-password-button.html > > added in https://trac.webkit.org/changeset/238545/webkit > > is timing out at around 300 seconds on Mojave WK2. > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock- > indicator-should-be-visible-after-hiding-auto-fill-strong-password-button. > html Fixing

Daniel Bates

Comment 13 2018-11-27 13:22:59 PST

(In reply to Daniel Bates from comment #12) > (In reply to Truitt Savell from comment #11) > > The new test > > fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-after- > > hiding-auto-fill-strong-password-button.html > > > > added in https://trac.webkit.org/changeset/238545/webkit > > > > is timing out at around 300 seconds on Mojave WK2. > > > > History: > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=fast%2Fforms%2Fauto-fill-button%2Fcaps-lock- > > indicator-should-be-visible-after-hiding-auto-fill-strong-password-button. > > html > > Fixing Committed fix in <https://trac.webkit.org/changeset/238571>.

Note You need to log in before you can comment on or make changes to this bug.