WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
105903
Clean up the loadXXXStyle() idiom in StyleResolver.
https://bugs.webkit.org/show_bug.cgi?id=105903
Summary
Clean up the loadXXXStyle() idiom in StyleResolver.
Mike West
Reported
2013-01-01 16:39:59 PST
See
https://bugs.webkit.org/show_bug.cgi?id=90834#c11
for details.
Attachments
Patch
(5.15 KB, patch)
2013-01-02 02:00 PST
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch for landing
(3.63 KB, patch)
2013-01-02 12:57 PST
,
Mike West
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2013-01-01 20:57:28 PST
> See
https://bugs.webkit.org/show_bug.cgi?id=90834#c11
for details.
Can you post adequate information here? It's a simple copy/paste for you, but extra effort for multiple other people to do the research.
Mike West
Comment 2
2013-01-02 01:14:15 PST
(In reply to
comment #1
)
> > See
https://bugs.webkit.org/show_bug.cgi?id=90834#c11
for details. > > Can you post adequate information here? It's a simple copy/paste for you, but extra effort for multiple other people to do the research.
Very fair. This was meant as a note to myself, but you're absolutely right; the bug tracker should be a source of truth. --- In
https://bugs.webkit.org/show_bug.cgi?id=90834#c11
, akling@ suggested:
> Source/WebCore/css/StyleResolver.cpp:1361 > + if (!defaultSeamlessStyle) > + loadSeamlessStyle(); > + matchUARules(result, defaultSeamlessStyle);
This pattern seems overly verbose, it would be nicer if we could just do: matchUARules(result, defaultSeamlessStyle()); (I see that we already have code like this elsewhere in StyleResolver.cpp, but I think we can do better.)
> Source/WebCore/css/StyleResolver.cpp:5304 > + if (!defaultSeamlessStyle) > + loadSeamlessStyle(); > + m_features.add(defaultSeamlessStyle->features());
Again, would be nicer with just: m_features.add(defaultSeamlessStyle()->features());
Mike West
Comment 3
2013-01-02 02:00:24 PST
Created
attachment 181018
[details]
Patch
Mike West
Comment 4
2013-01-02 02:01:29 PST
akling@: Hopefully this is more or less what you were looking for. What do you think?
Darin Adler
Comment 5
2013-01-02 10:36:46 PST
Comment on
attachment 181018
[details]
Patch Looks OK. Also not sure that adding “UA” to these function names improved anything. It just seems like mysterious additional acronym/jargon. Not sure that it even helps in the name matchUARules.
Mike West
Comment 6
2013-01-02 12:29:19 PST
(In reply to
comment #5
)
> (From update of
attachment 181018
[details]
) > Looks OK.
Thanks. I'm dropping the seamless bit from the patch, as that got rolled back and it looks like we'll be doing things entirely differently. The view source bit is still relevant, however, so I appreciate the review.
> Also not sure that adding “UA” to these function names improved anything. It just seems like mysterious additional acronym/jargon. Not sure that it even helps in the name matchUARules.
In context, I think you're right. I'll drop UA from this method name. I don't think I want to touch matchUARules, however. I'll leave that decision for someone who knows what they're doing. :)
Mike West
Comment 7
2013-01-02 12:57:25 PST
Created
attachment 181050
[details]
Patch for landing
WebKit Review Bot
Comment 8
2013-01-02 13:15:13 PST
Comment on
attachment 181050
[details]
Patch for landing Clearing flags on attachment: 181050 Committed
r138639
: <
http://trac.webkit.org/changeset/138639
>
WebKit Review Bot
Comment 9
2013-01-02 13:15:17 PST
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