WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
65738
[meta] User agent shadow node with -webkit-user-modify:read-write causes problems
https://bugs.webkit.org/show_bug.cgi?id=65738
Summary
[meta] User agent shadow node with -webkit-user-modify:read-write causes prob...
Kent Tamura
Reported
2011-08-04 19:31:46 PDT
This is very similar to
Bug 65362
. 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.
Attachments
Patch
(6.43 KB, patch)
2012-03-06 00:15 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch
(6.43 KB, patch)
2012-03-06 00:41 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch
(10.99 KB, patch)
2012-03-06 01:59 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2011-08-04 19:35:35 PDT
(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.
Ryosuke Niwa
Comment 2
2011-08-04 19:45:55 PDT
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.
Kent Tamura
Comment 3
2011-08-04 20:08:42 PDT
(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.
Kent Tamura
Comment 4
2011-08-04 20:08:59 PDT
(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.
Ryosuke Niwa
Comment 5
2011-08-04 20:14:04 PDT
(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"?
Kent Tamura
Comment 6
2011-08-04 21:24:14 PDT
(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.
Ryosuke Niwa
Comment 7
2011-08-04 22:51:05 PDT
(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?
Kent Tamura
Comment 8
2011-08-04 23:01:27 PDT
(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.
Ryosuke Niwa
Comment 9
2011-08-04 23:25:32 PDT
(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.
Hajime Morrita
Comment 10
2012-03-06 00:15:19 PST
Created
attachment 130313
[details]
Patch
Kent Tamura
Comment 11
2012-03-06 00:20:42 PST
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?
Hajime Morrita
Comment 12
2012-03-06 00:41:30 PST
Created
attachment 130318
[details]
Patch
Hajime Morrita
Comment 13
2012-03-06 00:43:27 PST
(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?
Kent Tamura
Comment 14
2012-03-06 00:48:16 PST
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.
Hajime Morrita
Comment 15
2012-03-06 01:59:02 PST
Created
attachment 130337
[details]
Patch
Kent Tamura
Comment 16
2012-03-06 02:14:37 PST
Comment on
attachment 130337
[details]
Patch ok
WebKit Review Bot
Comment 17
2012-03-06 03:02:56 PST
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
Hajime Morrita
Comment 18
2012-03-06 20:18:32 PST
Comment on
attachment 130337
[details]
Patch Found this doesn't work. ews is correct.
Eric Seidel (no email)
Comment 19
2012-03-27 12:45:07 PDT
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
.
Shinya Kawanaka
Comment 20
2012-07-24 19:12:56 PDT
Fixing input element is harder than others. Let's try to fix element by element...
Shinya Kawanaka
Comment 21
2012-07-24 23:18:21 PDT
Oh, I found a bug of the suggested patch. We should not have -webkit-user-modify: read-only for <input> or <button> itself!!
Shinya Kawanaka
Comment 22
2012-07-24 23:19:40 PDT
(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...
Shinya Kawanaka
Comment 23
2012-08-06 21:49:01 PDT
Since all dependent bugs are closed, let's close this also. If you think it's not appropriate, please feel free to reopen.
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