Summary: | Caret disappears at end of password field when caps lock indicator is shown; password field not scrolled when caps lock indicator is shown | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||
Component: | HTML Editing | Assignee: | Daniel Bates <dbates> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dino, simon.fraser, webkit-bug-importer, wenson_hsieh, zalan | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Local Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
URL: | data:text/html,%20<input%20type="password"%20style="width:50px"> | ||||||||
Bug Depends on: | 191968 | ||||||||
Bug Blocks: | 190571 | ||||||||
Attachments: |
|
Description
Daniel Bates
2018-11-01 12:24:24 PDT
We do not seem to auto scroll the password field when the caps lock indicator is shown. 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.
Created attachment 355651 [details]
Patch and layout test
Comment on attachment 355651 [details] Patch and layout test Patch will not apply to trunk as it depends on the patch for bug #191968. 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. 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. Committed r238522: <https://trac.webkit.org/changeset/238522> |