Search and Top Navigation
#14917 closed bug (fixed)
Opened February 11, 2016 10:50AM UTC
Closed February 16, 2016 05:22PM UTC
Autocomplete does not recognize contenteditable on chromium browsers
Reported by: | shokkobon | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.12.0 |
Component: | ui.autocomplete | Version: | 1.11.4 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
If we add autocomplete to a hidden element that has contenteditable attribute set in chromium browsers autocomplete will not recognize that the element has contenteditable attribute enabled. This will cause buggy behaviour such as html content being replaced with text value.
I've tested the bug in:
- Chrome Version 48.0.2564.109 m OS: Win 8.1
I've tested with:
- jquery.js 1.12.0 & jquery-ui.js 1.11.4
- jquery.js 2.2.0 & jquery-ui.js 1.11.4
The is the relevant chrome issue:
This is a jsbin where you can see the issue:
- jquery 1.12.0 https://jsbin.com/rinilasupu/1/edit?html,js,output
- jquery 2.2.0 https://jsbin.com/hebipeqozo/edit?html,js,output
To reproduce:
- use chrome
- click 'toggle'
- write 'He' in div that appears
- once dropdown appears press 'UP' key three times (so focus goes back to contenteditable element)
- observe caret moved to the beggining of element (element's content has been replaced)
The problem is that this code fails:
line 85 of autocomplete.js
#!js this.isMultiLine = isTextarea || !isInput && this.element.prop( "isContentEditable" );
The chromium bug 313082 will result in
this.element.prop( "isContentEditable" )to return
false for hidden elements with contenteditable attribute set.
A possible solution is to change that line to:
#!js this.isMultiLine = isTextarea || !isInput && (this.element.prop( "contentEditable" ) === "true");
as this check will return "true" correctly even in chromium browsers.
This is the proposed change:
I did not make the PR as I'm unsure of the code quality and the procedure
for PR's with jquery ui.
Attachments (0)
Change History (4)
Changed February 15, 2016 03:56PM UTC by comment:1
status: | new → open |
---|
Changed February 15, 2016 08:44PM UTC by comment:2
I'm afraid it may be a while until we see a fix in chromium as this bug has been open since October 2013 and it's currently unassigned. It's unfortunate that Blink is the last engine to sport this bug. The CKEditor comment that believes that the bug will be resolved soon in Blink is over 30 months old!
I know that this situation is very specific, using autocomplete on a hidden contenteditable, but it may be a possible scenario for single page applications where parts of the dom may be hidden until needed.
I did not think of inheritance. If I understand correctly the originally proposed code would fail for all browsers in cases where an element has inherted the contenteditable property?
May I suggest a possible solution that will not cover all the chrome bug cases but should provide some relief for chrome users and would still work on other browsers at the expense of an another check:
#!js this.isMultiLine = isTextarea || !isInput && (this.element.prop( "isContentEditable" ) || this.element.prop( "contentEditable" ) === "true");
This would fix the bug in chromium browser in instances where the element on which the autocomplete is applied has the contenteditable property set directly.
The fix seems ugly though and would add code that exists only because of a specific browser bug. Also, once the chromium issue is resolved the fix should be reverted, so this would add a bit of debt to the codebase. And I agree that this may not be a widespread problem.
Changed February 16, 2016 03:23PM UTC by comment:3
Changed February 16, 2016 05:22PM UTC by comment:4
milestone: | none → 1.12.0 |
---|---|
resolution: | → fixed |
status: | open → closed |
Fixed in cbceca7091b17e05c5a9ba887ef54761568bb70b.
Using
this.element.prop( "contentEditable" ) === "true"
doesn't account for inheritance. The check would certainly be possible, but would require DOM traversal. Considering that even CKEditor has decided to just wait for Chromium to be fixed, I'm not sure that this is a wide spread problem.