474878 - Implement display of .indeterminate property on Windows

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Michael Ventnor]

Windows needs support for displaying the .indeterminate DOM property in its native checkboxes. Indeterminate checkboxes are the kinds of checkboxes you get at the top of a list when only some of the checkboxes within that list are ticked.

Comment 1

[Sylvain Pasche]

Attachment: DrawThemeBackground part.

work in progress, DrawFrameControl still needs to be done (ClassicGetThemePartAndState). Found this sample which can be useful: http://www.jeskola.net/buzz/beta/files/dev/patternxp/tree/CreateCheckboxImageList.cpp.

Comment 2

[Sylvain Pasche]

Attachment: Windows .indeterminate implementation

This depends on the patch on bug 474868.

Using GetCheckedOrSelected() could save a few duplicated code, I tried to keep the old logic.

Robert, do you want to review this, or do you suggest someone else?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+      enum InputState {
+        UNCHECKED = 0, CHECKED, INDETERMINATE
+      } inputState = UNCHECKED;

Prefer to separate the declaration of InputState from the declaration of inputState.

+        if (GetCheckedOrSelected(aFrame, !isCheckbox))
+          inputState = CHECKED;
+        if (isCheckbox && GetIndeterminate(aFrame))
+          inputState = INDETERMINATE;

{} around these conditional statements. We generally use {} unless the statement transfers control (e.g. break/continue/return).

Please post a new patch with that. Thanks!!!

Comment 4

[Sylvain Pasche]

Attachment: Windows .indeterminate implementation v2

(In reply to comment #3)
> +      enum InputState {
> +        UNCHECKED = 0, CHECKED, INDETERMINATE
> +      } inputState = UNCHECKED;
> 
> Prefer to separate the declaration of InputState from the declaration of
> inputState.

done

> +        if (GetCheckedOrSelected(aFrame, !isCheckbox))
> +          inputState = CHECKED;
> +        if (isCheckbox && GetIndeterminate(aFrame))
> +          inputState = INDETERMINATE;
> 
> {} around these conditional statements. We generally use {} unless the
> statement transfers control (e.g. break/continue/return).

did that for those two and a few others near things I touched.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/557cfff414c6. Thanks!!