[webkit-reviews] review denied: [Bug 9589] WCSS: -wap-accesskey not supported : [Attachment 34197] resubmit the patch with a more descriptive ChangeLog following the review comments
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 6 16:38:38 PDT 2009
Eric Seidel <eric at webkit.org> has denied Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 9589: WCSS: -wap-accesskey not supported
https://bugs.webkit.org/show_bug.cgi?id=9589
Attachment 34197: resubmit the patch with a more descriptive ChangeLog
following the review comments
https://bugs.webkit.org/attachment.cgi?id=34197&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
Why does WebKit want this?
This seems like a non-critical feature of an obscure spec. This if for mobile
no? I don't know of a mobile device with an alt key, but I believe you there
is one. How many pages on the WAP web actually depend on this parsing? Is
there a CSS3 equivilant that we can just map this property to?
You change to the ChangeLog to tell me to read the spec, isn't very helpful.
Reviewing is often hard. There are 80 patches in the queue right now. Patches
which are easy to r+ are the ones which get reviewed. In this case, it's not
obvious why this is a good thing for the project, why it's correct, or even
what it does. There is a relatively large amount of code to review in this
patch (2-3 pages of code).
I'm not trying to be difficult. But if you want this reviewed, you need to make
it easy for the reviewers to r+ it. Much of that work is on selling your
patch. making it small, making the ChangeLog and tests super-clear. Patches
which aren't easy to r+ just sit in the queue for forever, until someone like
me comes along and r-'s them.
Also, I don't think you want to make me the requestee. :) That's likely only
to slow down your reviews as others will ignore them.
Please add more useful information to the ChangeLog, including answering some
of the questions I posed above.
More information about the webkit-reviews
mailing list