Bug 79754 - [Forms] Spin button sometimes ignores Indeterminate of m_upDownState
Summary: [Forms] Spin button sometimes ignores Indeterminate of m_upDownState
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-27 23:46 PST by yosin
Modified: 2012-02-28 22:17 PST (History)
2 users (show)

See Also:


Attachments
Patch 1 (2.30 KB, patch)
2012-02-28 00:43 PST, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (2.44 KB, patch)
2012-02-28 02:05 PST, yosin
no flags Details | Formatted Diff | Diff
Patch 3 (5.55 KB, patch)
2012-02-28 18:13 PST, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-02-27 23:46:39 PST
SpinButton implementation contains code like below:
  input->stepUpFromRenderer(m_upDownState == Up ? 1 : -1);

Member variable m_upDownState is type of enum UpDownState which has three values, Indeterminate, Down, and Up.
Above code can pass -1 to input->setUpFromRenderer if m_upDownState is Down or Indeterminate. We should not call input->setUpFromRenderer if m_upDownState is Indeterminate.
Comment 1 yosin 2012-02-28 00:43:37 PST
Created attachment 129209 [details]
Patch 1
Comment 2 Kent Tamura 2012-02-28 01:51:49 PST
Comment on attachment 129209 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=129209&action=review

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:-297
> -                ASSERT(m_upDownState != Indeterminate);

If you think this assertion should be removed, please write a reason in ChangeLog.

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:299
> +                if (m_upDownState != Indeterminate) {
> +                  input->stepUpFromRenderer(m_upDownState == Up ? 1 : -1);
> +                  if (renderer())

Wrong indentation.
Comment 3 yosin 2012-02-28 02:05:34 PST
Created attachment 129217 [details]
Patch 2
Comment 4 Kent Tamura 2012-02-28 02:33:11 PST
Comment on attachment 129217 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=129217&action=review

> Source/WebCore/ChangeLog:14
> +        m_upDownState can be Indeterminate at mousedown event if mouse pointer is on
> +        spin button when it is displayed.
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        No new tests. No behavior change.

This patch changes a behavior, and it seems you know how to reproduce the behavior change.  Please write a test.

Also, usual position of "Reviewed by .." line is just after the bug URL, before the long description.
Comment 5 yosin 2012-02-28 18:13:07 PST
Created attachment 129369 [details]
Patch 3
Comment 6 Kent Tamura 2012-02-28 18:21:27 PST
Comment on attachment 129369 [details]
Patch 3

Looks good.  Thanks!
Comment 7 WebKit Review Bot 2012-02-28 22:17:25 PST
Comment on attachment 129369 [details]
Patch 3

Clearing flags on attachment: 129369

Committed r109193: <http://trac.webkit.org/changeset/109193>
Comment 8 WebKit Review Bot 2012-02-28 22:17:30 PST
All reviewed patches have been landed.  Closing bug.