WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
191164
Caret disappears at end of password field when caps lock indicator is shown; password field not scrolled when caps lock indicator is shown
https://bugs.webkit.org/show_bug.cgi?id=191164
Summary
Caret disappears at end of password field when caps lock indicator is shown; ...
Daniel Bates
Reported
2018-11-01 12:24:24 PDT
Note you will need a build of WebKit with the fix for
bug #190056
(if it has not already landed) to reproduce in a WebKit2 app, such as Safari. Alternatively, you can reproduce using ToT WebKit using a WebKit1 app (say, via MiniBrowser). Perform the following: 1. Visit <data:text/html,%20<input%20type="password"%20style="width:50px">>. 2. Focus the password field and type enough characters to fill the field. 3. Press the caps lock key on the keyboard. Then the caret will no longer be visible.
Attachments
For for Mac, but does not fix caret on iOS :(
(614 bytes, patch)
2018-11-02 16:48 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch and layout test
(16.42 KB, patch)
2018-11-26 10:32 PST
,
Daniel Bates
dino
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2018-11-01 12:25:21 PDT
We do not seem to auto scroll the password field when the caps lock indicator is shown.
Radar WebKit Bug Importer
Comment 2
2018-11-01 12:32:46 PDT
<
rdar://problem/45738179
>
Daniel Bates
Comment 3
2018-11-02 16:48:21 PDT
Created
attachment 353744
[details]
For for Mac, but does not fix caret on iOS :( Attached patch that fixes this issue on Mac. For some reason it does not fix the issue on iOS. Keep in mind that the bad behavior on iOS is that the caret is drawn at the wrong position whereas on Mac the "caret disappears". The different in bad behavior is due to the fact that the caret on iOS is painted by UIKit on top of the WebView.
Daniel Bates
Comment 4
2018-11-26 10:32:09 PST
Created
attachment 355651
[details]
Patch and layout test
Daniel Bates
Comment 5
2018-11-26 10:32:53 PST
Comment on
attachment 355651
[details]
Patch and layout test Patch will not apply to trunk as it depends on the patch for
bug #191968
.
Dean Jackson
Comment 6
2018-11-26 12:00:01 PST
Comment on
attachment 355651
[details]
Patch and layout test View in context:
https://bugs.webkit.org/attachment.cgi?id=355651&action=review
> Source/WebCore/editing/FrameSelection.h:162 > + void setNeedsSelectionUpdate(bool forceRevealSelection = false);
Suggestion: declare an enum class RevealSelection { NotForced, Forced }; and use that as the parameter.
> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:219 > + frame().selection().setNeedsSelectionUpdate(true /* forceRevealSelection */);
... that way you don't need this comment.
> LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html:45 > +function runNextTest() > +{ > + if (currentTest >= tests.length) { > + done(); > + return; > + } > + tests[currentTest++](); > +}
Suggestion: declare const tests = [ function testFocusedEmptyFieldIsNotScrolled() { ... }, function testFieldDidNotScrollAfterTyping() { ... }, ... ]; Then you just need function runTests() { tests.forEach(t => { t(); }); done(); } Saves the nested stack in runNextTest() as well as all the calls to it.
Daniel Bates
Comment 7
2018-11-26 12:51:13 PST
Comment on
attachment 355651
[details]
Patch and layout test View in context:
https://bugs.webkit.org/attachment.cgi?id=355651&action=review
>> Source/WebCore/editing/FrameSelection.h:162 >> + void setNeedsSelectionUpdate(bool forceRevealSelection = false); > > Suggestion: declare an enum class RevealSelection { NotForced, Forced }; and use that as the parameter.
Will update the patch based on your suggestion before landing. For completeness, I thought about adding such an enum when writing this patch. I chose not to since there is exactly one caller that passes forceRevealSelection := true. Maybe in the future there will be more such callers. Regardless, will switch to using an enum instead of a boolean.
>> LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html:45 >> +} > > Suggestion: declare const tests = [ > function testFocusedEmptyFieldIsNotScrolled() { ... }, > function testFieldDidNotScrollAfterTyping() { ... }, > ... > ]; > > Then you just need > function runTests() { > tests.forEach(t => { t(); }); > done(); > } > > Saves the nested stack in runNextTest() as well as all the calls to it.
This structure assumes that each test executes synchronously such that once we return from the call to t_i() we can call t_{i + 1}(), ..., t_N(). This is not case for most of these sub-tests. We typically need to wait for DOM event to be dispatched before we can move on to the next test.
Daniel Bates
Comment 8
2018-11-26 14:15:24 PST
Committed
r238522
: <
https://trac.webkit.org/changeset/238522
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug