Bug 65738

(In reply to comment #0) > 1. Load data:text/html,<style>input::-webkit-textfield-decoration-container { -webkit-user-modify: read-write; }</style><input type=search> > 2. Focus on the search field > 3. Press backward-delete key > 4. Crash > > Another related issue: > 1. Load data:text/html,<style>input::-webkit-search-results-button { -webkit-user-modify: read-write; }</style><input type=search results=20> > 2. Right-click on the magnifier button. > 3. You can type into the magnifier button. I don't think we should allow these styles.

I mean the problem is that inputType's are holding onto nodes that have been removed from the document, and RenderTextControlSingleLine is blowing up because it assumes that they have renderer (which they have also when they were removed from the document). The only way we could prevent this crash is if we either detected those cases where buttons were edited by the user and we have to ignore them in RenderTextControlSingleLine or editing code became aware of nodes that may be special for input types and avoided deleting/moving them. I don't think the latter is realistic in that there are so many places this could happen. The former is possible but I don't think it's really worth the effort because there are so many places where we depend on the existence of those nodes.

(In reply to comment #1) > I don't think we should allow these styles. Yes. The fix would be to add -webkit-user-modify:read-only to html.css.

(In reply to comment #3) > (In reply to comment #1) > > I don't think we should allow these styles. > > Yes. The fix would be to add -webkit-user-modify:read-only to html.css. I meant -webkit-user-modify:read-only !important.

(In reply to comment #4) > I meant -webkit-user-modify:read-only !important. Would that still work if author added "-webkit-user-modify: read-write !important"?

(In reply to comment #5) > (In reply to comment #4) > > I meant -webkit-user-modify:read-only !important. > > Would that still work if author added "-webkit-user-modify: read-write !important"? Authors and users can't change values with !important. Other examples: data:text/html,<style>progress { -webkit-appearance: none; } progress::-webkit-progress-bar {-webkit-user-modify:read-write; }</style><progress value=50 max=100></progress> data:text/html,<style>meter { -webkit-appearance: none; } meter::-webkit-meter-bar {-webkit-user-modify:read-write; }</style><meter value=50 max=100></meter> We can type into <progress> and <meter>, and can remove the value block.

(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > I meant -webkit-user-modify:read-only !important. > > > > Would that still work if author added "-webkit-user-modify: read-write !important"? > > Authors and users can't change values with !important. Okay. Great! But we should definitely have a test for that just in case. > Other examples: > > data:text/html,<style>progress { -webkit-appearance: none; } progress::-webkit-progress-bar {-webkit-user-modify:read-write; }</style><progress value=50 max=100></progress> > data:text/html,<style>meter { -webkit-appearance: none; } meter::-webkit-meter-bar {-webkit-user-modify:read-write; }</style><meter value=50 max=100></meter> > > We can type into <progress> and <meter>, and can remove the value block. Should I write a patch, or will you?

(In reply to comment #7) > Should I write a patch, or will you? I don't think you should write a patch. I feel this issue is in the shadow DOM area, rather than HTML Editing or Forms.

(In reply to comment #8) > (In reply to comment #7) > > Should I write a patch, or will you? > > I don't think you should write a patch. > I feel this issue is in the shadow DOM area, rather than HTML Editing or Forms. Sure. Although I've gotten quite familiar with all these shadow DOM stuff by now.

Created attachment 130313 [details] Patch

Comment on attachment 130313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130313&action=review > Source/WebCore/html/shadow/TextControlInnerElements.cpp:72 > + RefPtr<RenderStyle> style = document()->styleSelector()->styleForElement(this, 0, true); > + style->setUserModify(READ_ONLY); Is it safe to update a shared RenderStyle?

Created attachment 130318 [details] Patch

(In reply to comment #11) > Is it safe to update a shared RenderStyle? Good question. No! We need a fresh RenderStyle object. I updated the patch. Could you take another look please?

Comment on attachment 130318 [details] Patch BTW, I feel this is an overkill. Adding -webkit-user-modify: read-only !important; to the UA stylesheet is enough.

Created attachment 130337 [details] Patch

Comment on attachment 130337 [details] Patch ok

Comment on attachment 130337 [details] Patch Attachment 130337 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11835296 New failing tests: editing/selection/3690703-2.html editing/execCommand/button.html editing/selection/4397952.html

Comment on attachment 130337 [details] Patch Found this doesn't work. ews is correct.

Comment on attachment 130337 [details] Patch Cleared Kent Tamura's review+ from obsolete attachment 130337 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Fixing input element is harder than others. Let's try to fix element by element...

Oh, I found a bug of the suggested patch. We should not have -webkit-user-modify: read-only for <input> or <button> itself!!

(In reply to comment #21) > Oh, I found a bug of the suggested patch. > We should not have -webkit-user-modify: read-only for <input> or <button> itself!! Any way, I'll try to fix this bug one by one...

Since all dependent bugs are closed, let's close this also. If you think it's not appropriate, please feel free to reopen.