WCSS input extension is one of the WAP specific extenstions to CSS2. It is consisted of two WCSS properties: -wap-input-format and -wap-input-required
Created attachment 27391 [details] inital patch for WCSS input support
Created attachment 27653 [details] updated patch for changed code split the initial patch into two parts: this one contains the changed code, the upcoming one is for layout test
Created attachment 27654 [details] updated patch for layout test
Created attachment 30315 [details] Updated patch for adding tests The latest tests for WCSS input extension
Created attachment 30316 [details] Updated patch about code changes Latest code changes for WCSS input extension support
Created attachment 30443 [details] Latest Updated patch for code change add copyright messages for code change
Comment on attachment 30315 [details] Updated patch for adding tests will need to be added to skipped for Mac OS also, but patch is fine.
(In reply to comment #7) > (From update of attachment 30315 [details] [review]) > will need to be added to skipped for Mac OS also, but patch is fine. Mac OS? I have changed platform/mac/Skipped. Do you mean other mac-xxx platforms?
Sorry, I meant Qt.
Comment on attachment 30443 [details] Latest Updated patch for code change > + This change depends on https://bugs.webkit.org/show_bug.cgi?id=23452 This doesn't make sense to have in a changelog. Either the change works or it doesn't. > + > + Tests: fast/wcss/inputelement-inputformat.xhtml > + fast/wcss/inputelement-inputrequired.xhtml > + fast/wcss/textarea-inputformat.xhtml > + fast/wcss/textarea-inputrequired.xhtml These tests don't seem to be included. > + bool ok = true; > + UChar mask = inputFormatMask[maskIndex]; > + // Match the inputed character with input mask > + switch (mask) { > + case 'A': > + ok = !WTF::isASCIIDigit(inChar) && !WTF::isASCIILower(inChar) && WTF::isASCIIPrintable(inChar); > + break; > + case 'a': > + ok = !WTF::isASCIIDigit(inChar) && !WTF::isASCIIUpper(inChar) && WTF::isASCIIPrintable(inChar); > + break; > + case 'N': > + ok = WTF::isASCIIDigit(inChar); > + break; > + case 'n': > + ok = !WTF::isASCIIAlpha(inChar) && WTF::isASCIIPrintable(inChar); > + break; > + case 'X': > + ok = !WTF::isASCIILower(inChar) && WTF::isASCIIPrintable(inChar); > + break; > + case 'x': > + ok = !WTF::isASCIIUpper(inChar) && WTF::isASCIIPrintable(inChar); > + break; > + case 'M': > + ok = WTF::isASCIIPrintable(inChar); > + break; > + case 'm': > + ok = WTF::isASCIIPrintable(inChar); The WTF should not be needed for these (or at the very least, you should add using namespace WTF at the top of file). The cases for 'M' and 'm' can be combined. > Index: WebCore/html/HTMLTextAreaElement.h > =================================================================== > --- WebCore/html/HTMLTextAreaElement.h (revision 43681) > +++ WebCore/html/HTMLTextAreaElement.h (working copy) > @@ -3,6 +3,7 @@ > * (C) 1999 Antti Koivisto (koivisto@kde.org) > * (C) 2000 Dirk Mueller (mueller@kde.org) > * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved. > + * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/) > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Library General Public > @@ -26,11 +27,19 @@ > > #include "HTMLFormControlElement.h" > > +#if ENABLE(WCSS) > +#include "InputElement.h" > +#endif > + > namespace WebCore { > > class VisibleSelection; > > +#if ENABLE(WCSS) > +class HTMLTextAreaElement : public HTMLFormControlElementWithState, public InputElement { > +#else > class HTMLTextAreaElement : public HTMLFormControlElementWithState { > +#endif This seems wrong to me. An HTMLTextAreaElement is not an InputElement and therefore should never inherit from InputElement. The changes made to this class seem too invasive and I think a cleaner approach is necessary. r-
(In reply to comment #10) > (From update of attachment 30443 [details] [review]) > > + This change depends on https://bugs.webkit.org/show_bug.cgi?id=23452 > This doesn't make sense to have in a changelog. Either the change works or it > doesn't. > > + > > + Tests: fast/wcss/inputelement-inputformat.xhtml > > + fast/wcss/inputelement-inputrequired.xhtml > > + fast/wcss/textarea-inputformat.xhtml > > + fast/wcss/textarea-inputrequired.xhtml > These tests don't seem to be included. will remove it. > > + bool ok = true; > > + UChar mask = inputFormatMask[maskIndex]; > > + // Match the inputed character with input mask > > + switch (mask) { > > + case 'A': > > + ok = !WTF::isASCIIDigit(inChar) && !WTF::isASCIILower(inChar) && WTF::isASCIIPrintable(inChar); > > + break; > > + case 'a': > > + ok = !WTF::isASCIIDigit(inChar) && !WTF::isASCIIUpper(inChar) && WTF::isASCIIPrintable(inChar); > > + break; > > + case 'N': > > + ok = WTF::isASCIIDigit(inChar); > > + break; > > + case 'n': > > + ok = !WTF::isASCIIAlpha(inChar) && WTF::isASCIIPrintable(inChar); > > + break; > > + case 'X': > > + ok = !WTF::isASCIILower(inChar) && WTF::isASCIIPrintable(inChar); > > + break; > > + case 'x': > > + ok = !WTF::isASCIIUpper(inChar) && WTF::isASCIIPrintable(inChar); > > + break; > > + case 'M': > > + ok = WTF::isASCIIPrintable(inChar); > > + break; > > + case 'm': > > + ok = WTF::isASCIIPrintable(inChar); > The WTF should not be needed for these (or at the very least, you should add > using namespace WTF at the top of file). The cases for 'M' and 'm' can be > combined. Thanks, will combine them. > > Index: WebCore/html/HTMLTextAreaElement.h > > =================================================================== > > --- WebCore/html/HTMLTextAreaElement.h (revision 43681) > > +++ WebCore/html/HTMLTextAreaElement.h (working copy) > > @@ -3,6 +3,7 @@ > > * (C) 1999 Antti Koivisto (koivisto@kde.org) > > * (C) 2000 Dirk Mueller (mueller@kde.org) > > * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved. > > + * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/) > > * > > * This library is free software; you can redistribute it and/or > > * modify it under the terms of the GNU Library General Public > > @@ -26,11 +27,19 @@ > > > > #include "HTMLFormControlElement.h" > > > > +#if ENABLE(WCSS) > > +#include "InputElement.h" > > +#endif > > + > > namespace WebCore { > > > > class VisibleSelection; > > > > +#if ENABLE(WCSS) > > +class HTMLTextAreaElement : public HTMLFormControlElementWithState, public InputElement { > > +#else > > class HTMLTextAreaElement : public HTMLFormControlElementWithState { > > +#endif > This seems wrong to me. An HTMLTextAreaElement is not an InputElement and > therefore should never inherit from InputElement. The changes made to this > class seem too invasive and I think a cleaner approach is necessary. > r- In my opinion, HTMLTextAreaElement can be treated as an InputElement since it also receives user input. Does any specifications define InputElement explicitly and say HTMLTextAreaElement is not an InputElment? I am not sure for that.
I'm confused. Is there something to be landed here? If not, can we remove from the commit queue?
well, the tests seem fine. I guess they could be landed (disabled)
Landing tests without the supporting functionality isn't hugely useful, and would require a different patch than the one that is attached.
Comment on attachment 30315 [details] Updated patch for adding tests Clearing the review flag to get this out of the review queue for now.
Created attachment 38967 [details] re-submit the patch after porting to ToT and addressing the reviews comments patch that addresses the reviewer's comments. This patch is to add WCSS input extension to CSS2, which includes the following CSS properties: -wap-input-format -wap-input-required with -wap-input-format, the author can define the value space of an input element. -wap-input-required says the input element can't be empty
Given that we have failed to find a reviewer to support this in over 2 months, and that I am no longer in support of adding this feature to WebKit, I think we should close out this bug/patch due to lack of reviewer support.
Comment on attachment 38967 [details] re-submit the patch after porting to ToT and addressing the reviews comments r- due to lack of reviewer support based on my above comment and that no one has commented on this bug in the 2 weeks since I made it.
Comment on attachment 38967 [details] re-submit the patch after porting to ToT and addressing the reviews comments 6 weeks later, same story. WebKit's apathy towards this patch suggests we should close this bug as WONTFIX. Marking r- to remove this abandoned bug/patch from the review queue.
Bug 23477 was resolved with a general lack of desire to implement any WCSS extensions, so closing this one, too. Please e-mail webkit-dev mailing list if you are interested in WCSS support.