WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
90913
Regression(121518) TextFieldDecorationElement formatting is broken
https://bugs.webkit.org/show_bug.cgi?id=90913
Summary
Regression(121518) TextFieldDecorationElement formatting is broken
Garrett Casto
Reported
2012-07-10 14:51:41 PDT
Before this change, a TextFieldDecorationElement was rendered on the right hand side text field. After it, it is rendered on the left side of the text field.
Attachments
Linux UI
(58.91 KB, image/png)
2012-07-23 22:46 PDT
,
Garrett Casto
no flags
Details
Patch
(9.55 KB, patch)
2012-08-01 01:02 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Updated to ToT
(9.58 KB, patch)
2012-08-15 03:52 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Perf chart
(15.98 KB, text/html)
2012-08-15 03:55 PDT
,
Hajime Morrita
no flags
Details
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Hajime Morrita
Comment 1
2012-07-10 21:57:27 PDT
Hmm. How can I try that?
Garrett Casto
Comment 2
2012-07-11 10:23:13 PDT
Ah, sorry for not giving a way to reproduce. The only thing that I know of that uses this is a feature for generating passwords that I'm working on. 1) Add --enable-password-generation to the command line 2) Make sure that you are signed in with sync enabled for passwords. 3) Go to
https://myaccount.nytimes.com/register
There should be a small key icon in the first password field on the left side of the field. It should be on the right side. It's possible that there is an easier way to test this, but I'm not sure what it is.
Hajime Morrita
Comment 3
2012-07-22 22:17:43 PDT
I tried this on Chromium Linux ToT but I couldn't see any such icons. Am I doing something wrong?
Garrett Casto
Comment 4
2012-07-23 22:46:03 PDT
Created
attachment 153964
[details]
Linux UI
Garrett Casto
Comment 5
2012-07-23 23:00:59 PDT
One other reason it would not show up is if you've every clicked on the "never remember passwords for this site" infobar for nytimes.com. To be safe, I'd just trying using a clean profile. I've attached an image of how the UI should look in case it's not obvious when it shows up.
Hajime Morrita
Comment 6
2012-07-24 00:54:44 PDT
Oops. I accidentally used older binary. Now it shows the key icon. Thanks.
Hajime Morrita
Comment 7
2012-07-26 17:57:31 PDT
Created a reduction:
http://jsfiddle.net/
Hajime Morrita
Comment 8
2012-07-26 17:57:47 PDT
(In reply to
comment #7
)
> Created a reduction:
http://jsfiddle.net/
Oops:
http://jsfiddle.net/sggwj/
Hajime Morrita
Comment 9
2012-08-01 01:02:46 PDT
Created
attachment 155739
[details]
Patch
Hajime Morrita
Comment 10
2012-08-01 01:04:13 PDT
Hi Dimitri, could you take a look? This is happened to be a long awaited fix for me and Hayato-san :-)
Dimitri Glazkov (Google)
Comment 11
2012-08-01 09:34:59 PDT
Comment on
attachment 155739
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=155739&action=review
> Source/WebCore/dom/NodeRenderingContext.cpp:95 > + ComposedShadowTreeWalker walker(m_node);
This area is super-hot. What are the performance implications of this change?
Hajime Morrita
Comment 12
2012-08-02 00:47:50 PDT
(In reply to
comment #11
)
> (From update of
attachment 155739
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=155739&action=review
> > > Source/WebCore/dom/NodeRenderingContext.cpp:95 > > + ComposedShadowTreeWalker walker(m_node); > > This area is super-hot. What are the performance implications of this change?
True. I observed certain regression on Dromaeo. I'm going to return here with ComposedShadowTreeWalker speedup.
Hajime Morrita
Comment 13
2012-08-08 02:28:20 PDT
I want
Bug 59816
to make it faster. IsInShadow flag will help up a lot.
Eric Seidel (no email)
Comment 14
2012-08-12 03:37:08 PDT
Could you post the perf numbers to the bug? Would be good to document how bad this is for perf.
Hajime Morrita
Comment 15
2012-08-15 03:52:37 PDT
Created
attachment 158540
[details]
Updated to ToT
Hajime Morrita
Comment 16
2012-08-15 03:55:49 PDT
Created
attachment 158541
[details]
Perf chart Looks like I was wrong. Attached chart shows no significant slowdown and show even slight speedup. Now personally I'm rather optimistic and would like to land this to see how it affects in Page cycler speed.
Hajime Morrita
Comment 17
2012-08-15 03:57:45 PDT
(In reply to
comment #16
)
> Created an attachment (id=158541) [details] > Perf chart
First two are patch applied. Last two are original.
Dimitri Glazkov (Google)
Comment 18
2012-08-15 09:40:15 PDT
Comment on
attachment 158540
[details]
Updated to ToT ok. Not flipping the cq+, because I would prefer if you land it and watch it :)
Hajime Morrita
Comment 19
2012-08-15 19:24:26 PDT
Comment on
attachment 158540
[details]
Updated to ToT Fair enough. Let me flip it :-)
WebKit Review Bot
Comment 20
2012-08-15 20:06:15 PDT
Comment on
attachment 158540
[details]
Updated to ToT Clearing flags on attachment: 158540 Committed
r125739
: <
http://trac.webkit.org/changeset/125739
>
WebKit Review Bot
Comment 21
2012-08-15 20:06:20 PDT
All reviewed patches have been landed. Closing bug.
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