Bug 34783 - Fix two issues on maxLength.
Summary: Fix two issues on maxLength.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-09 21:04 PST by Kent Tamura
Modified: 2010-02-11 18:37 PST (History)
1 user (show)

See Also:


Attachments
Proposed patch (11.53 KB, patch)
2010-02-09 21:12 PST, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.2) (12.79 KB, patch)
2010-02-11 02:06 PST, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2010-02-09 21:04:58 PST
Fix two issues on maxLength.
Comment 1 Kent Tamura 2010-02-09 21:12:34 PST
Created attachment 48461 [details]
Proposed patch
Comment 2 Darin Adler 2010-02-10 10:49:35 PST
Comment on attachment 48461 [details]
Proposed patch

> +debug('<br />Non-dirty value');

It's better not to rely on markup in the debug function. I typically use debug('') to make a blank line. Someone should clean this up a bit for future tests.

The setNonDirtyValue affects a lot of different functions. There is not sufficient test coverage for this. Each function that you changed to call setNonDirtyValue needs a test that would fail if it was still calling setValue.

I'm going to say review- because of not enough test coverage. Everything else seems OK in the patch. If I'm wrong about the test coverage, please let me know.
Comment 3 Kent Tamura 2010-02-11 02:06:19 PST
Created attachment 48553 [details]
Proposed patch (rev.2)
Comment 4 Kent Tamura 2010-02-11 02:08:02 PST
(In reply to comment #2)
> (From update of attachment 48461 [details])
> > +debug('<br />Non-dirty value');

Fixed.

> The setNonDirtyValue affects a lot of different functions. There is not
> sufficient test coverage for this. Each function that you changed to call
> setNonDirtyValue needs a test that would fail if it was still calling setValue.

ok, I added tests to cover affected functions.
Comment 5 Kent Tamura 2010-02-11 18:37:33 PST
Comment on attachment 48553 [details]
Proposed patch (rev.2)

Clearing flags on attachment: 48553

Committed r54695: <http://trac.webkit.org/changeset/54695>
Comment 6 Kent Tamura 2010-02-11 18:37:48 PST
All reviewed patches have been landed.  Closing bug.