Aryan Krishnan (Gerrit)

unread,

May 31, 2026, 1:56:28 PMMay 31

to Helmut Januschka, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Helmut Januschka

Aryan Krishnan added 2 comments

Commit Message

Companion to the FWL_STYLE_WGT -> WidgetStyle Mask conversion. Convert the FWL widget state bits into a scoped WidgetState enum and store CFWL_Widget::Properties::states_ as Mask<WidgetState>.

Aryan Krishnan . unresolved

Wrap lines 9, 11, 13

File xfa/fwl/cfwl_datetimepicker.cpp

Line 29, Patchset 1 (Latest): Properties{{}, FWL_STYLEEXT_DTP_ShortDateFormat, {}},

Aryan Krishnan . unresolved

public:
    uint32_t styles_ = FWL_STYLE_WGT_Child;  // Mask of FWL_STYLE_*_*.
    uint32_t style_exts_ = 0;                // Mask of FWL_STYLEEXT_*_*.
    Mask<WidgetState> states_;

Shouldn't the first argument also be 0?
Although I do remember another CL did change the type of styles_.

Open in Gerrit

Related details

Attention is currently required from:

Helmut Januschka

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Jun 7, 2026, 5:12:52 PMJun 7

to Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Aryan Krishnan

Helmut Januschka added 2 comments

Commit Message

Line 9, Patchset 1:

Companion to the FWL_STYLE_WGT -> WidgetStyle Mask conversion. Convert the FWL widget state bits into a scoped WidgetState enum and store CFWL_Widget::Properties::states_ as Mask<WidgetState>.

Aryan Krishnan . resolved

Wrap lines 9, 11, 13

Helmut Januschka

Done

File xfa/fwl/cfwl_datetimepicker.cpp

Line 29, Patchset 1: Properties{{}, FWL_STYLEEXT_DTP_ShortDateFormat, {}},

Aryan Krishnan . resolved

public:
    uint32_t styles_ = FWL_STYLE_WGT_Child;  // Mask of FWL_STYLE_*_*.
    uint32_t style_exts_ = 0;                // Mask of FWL_STYLEEXT_*_*.
    Mask<WidgetState> states_;
Shouldn't the first argument also be 0?
Although I do remember another CL did change the type of styles_.

Helmut Januschka

awesome! styles_ is still uint32_t here, so only the third field (now Mask<WidgetState>) needed {}. Reverted the first arg back to 0, and fixed the same unnecessary change in cfwl_edit.cpp.

Open in Gerrit

Related details

Attention is currently required from:

Aryan Krishnan

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Jun 15, 2026, 7:09:33 PM (9 days ago) Jun 15

to Helmut Januschka, Lei Zhang, Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Aryan Krishnan, Helmut Januschka and Lei Zhang

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

Code-Review

+1

3 comments

File xfa/fwl/cfwl_caret.cpp

Line 28, Patchset 4 (Latest): SetStates(WidgetState::kCaretHighlight);

Tom Sepez . unresolved

Up to you, if you want to avoid the head-scratching between the plural name of the method and the singular variable name, you could write `SetStates({WidgetState::kCaretHighlight});` but do as you like.

File xfa/fwl/cfwl_edit.cpp

Line 584, Patchset 4 (Latest): if (properties_.states_ & WidgetState::kFocused && !rtCaret.IsEmpty()) {

Tom Sepez . unresolved

pre-existing:: suggest parens around & though not needed.

File xfa/fwl/cfwl_widget.h

Line 128, Patchset 4 (Latest): virtual void RemoveStates(Mask<WidgetState> states);

Tom Sepez . unresolved

pre-existing: It's always bugged me (but not enough to do anything about it) that set and remove aren't direct antonyms: set and clear, add and remove, but we're mixing the two. Want to change it to ClearStates while we at it?

Open in Gerrit

Related details

Attention is currently required from:

Aryan Krishnan
Helmut Januschka
Lei Zhang

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Jun 18, 2026, 4:24:06 PM (7 days ago) Jun 18

to Tom Sepez, Lei Zhang, Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Aryan Krishnan, Lei Zhang and Tom Sepez

Helmut Januschka added 3 comments

File xfa/fwl/cfwl_caret.cpp

Line 28, Patchset 4: SetStates(WidgetState::kCaretHighlight);

Tom Sepez . resolved

Up to you, if you want to avoid the head-scratching between the plural name of the method and the singular variable name, you could write `SetStates({WidgetState::kCaretHighlight});` but do as you like.

Helmut Januschka

Done

File xfa/fwl/cfwl_edit.cpp

Line 584, Patchset 4: if (properties_.states_ & WidgetState::kFocused && !rtCaret.IsEmpty()) {

Tom Sepez . resolved

pre-existing:: suggest parens around & though not needed.

Helmut Januschka

Done

File xfa/fwl/cfwl_widget.h

Line 128, Patchset 4: virtual void RemoveStates(Mask<WidgetState> states);

Tom Sepez . resolved

pre-existing: It's always bugged me (but not enough to do anything about it) that set and remove aren't direct antonyms: set and clear, add and remove, but we're mixing the two. Want to change it to ClearStates while we at it?

Helmut Januschka

for sure, always leave a place cleaner than you found it!

Open in Gerrit

Related details

Attention is currently required from:

Aryan Krishnan
Lei Zhang
Tom Sepez

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Jun 24, 2026, 2:41:48 PM (15 hours ago) Jun 24

to Helmut Januschka, Lei Zhang, Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Aryan Krishnan, Helmut Januschka and Lei Zhang

Tom Sepez voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Aryan Krishnan
Helmut Januschka
Lei Zhang

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Jun 24, 2026, 4:37:39 PM (13 hours ago) Jun 24

to Helmut Januschka, Lei Zhang, Tom Sepez, Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Aryan Krishnan and Helmut Januschka

Lei Zhang voted

Code-Review	+1
Commit-Queue	+2

Open in Gerrit

Related details

Attention is currently required from:

Aryan Krishnan
Helmut Januschka

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

unread,

Jun 24, 2026, 6:32:31 PM (12 hours ago) Jun 24

to Helmut Januschka, Lei Zhang, Tom Sepez, Aryan Krishnan, pdfium-...@googlegroups.com

pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change

Change information

Commit message:

Replace FWL_STATE defines with WidgetState enum and Mask



Companion to the FWL_STYLE_WGT -> WidgetStyle Mask conversion. Convert
the FWL widget state bits into a scoped WidgetState enum and store
CFWL_Widget::Properties::states_ as Mask<WidgetState>.

Includes the per-widget state values that share the same states_ field
(checkbox kCheckbox*/kHovered/kPressed, pushbutton kPushbuttonDefault,
caret kCaretHighlight). The pushbutton 'default' bit aliases the same
bit position as kCheckboxChecked, mirroring the existing layout where
the bit is reused for whichever widget owns the field at runtime.

SetStates(), RemoveStates(), and GetStates() are tightened to use
Mask<WidgetState>. Caret's local kStateHighlight constant moves into the
same enum as kCaretHighlight.

Bug: 42270078

Change-Id: Ic16c090c2dff87e1815d7a36aebc2c12731aaf57

Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/148835

Commit-Queue: Lei Zhang <the...@chromium.org>

Reviewed-by: Lei Zhang <the...@chromium.org>

Reviewed-by: Tom Sepez <tse...@chromium.org>

Files:

M xfa/fwl/cfwl_barcode.cpp
M xfa/fwl/cfwl_caret.cpp
M xfa/fwl/cfwl_checkbox.cpp
M xfa/fwl/cfwl_checkbox.h
M xfa/fwl/cfwl_combobox.cpp
M xfa/fwl/cfwl_combobox.h
M xfa/fwl/cfwl_comboedit.cpp
M xfa/fwl/cfwl_datetimeedit.cpp
M xfa/fwl/cfwl_datetimepicker.cpp
M xfa/fwl/cfwl_edit.cpp
M xfa/fwl/cfwl_edit.h
M xfa/fwl/cfwl_listbox.cpp
M xfa/fwl/cfwl_pushbutton.cpp
M xfa/fwl/cfwl_pushbutton.h
M xfa/fwl/cfwl_scrollbar.cpp
M xfa/fwl/cfwl_widget.cpp
M xfa/fwl/cfwl_widget.h
M xfa/fwl/cfwl_widgetmgr.cpp
M xfa/fxfa/cxfa_ffcheckbutton.cpp
M xfa/fxfa/cxfa_ffpushbutton.cpp

Change size: L

Delta: 20 files changed, 210 insertions(+), 208 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Lei Zhang, +1 by Tom Sepez

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Replace FWL_STATE defines with WidgetState enum and Mask [pdfium : main]

Aryan Krishnan (Gerrit)

Aryan Krishnan added 2 comments

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 2 comments

Related details

Tom Sepez (Gerrit)

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

3 comments

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 3 comments

Related details

Tom Sepez (Gerrit)

Tom Sepez voted Code-Review+1

Related details

Lei Zhang (Gerrit)

Lei Zhang voted

Related details

pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change

Change information