Page MenuHomeWildfire Games

Fix text input max_length attribute.
ClosedPublic

Authored by Stan on Oct 15 2019, 3:10 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23927: Fix text input max_length attribute. fixes rP15785
Summary

See #5266 caused by rP15785

Test Plan

Check any text fuzzing you now to make sure one cannot enter more than X characters in the fields with a max_length attribute.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Stan created this revision.Oct 15 2019, 3:10 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/974/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/459/display/redirect

Stan added a comment.EditedOct 15 2019, 3:17 PM

Original patch was made by Amplikon, but has been mostly rewritten.

To answer smiley's comments,

In #5266, @smiley wrote:

Use static_cast<int> instead of (int).

Done.

In #5266, @smiley wrote:

Unsigned integer probably.

int is used everywhere else except in one place where it uses long for some reason.

In #5266, @smiley wrote:

Sure it should not be a break;? (Same thing is in L308, Plus, from a quick skim, break; makes more sense to me. Could be wrong of course.)

Yep, because in this case if the function expect a return type, which was not easily visible without context.

Stan added a subscriber: smiley.Oct 15 2019, 3:18 PM
Stan removed a subscriber: smiley.
smiley added a comment.EditedOct 15 2019, 4:54 PM

Since apparently I had commented on this before, I have a feeling this would not quite work the way it should in certain cases.

Then again, its just a feeling. Consider that before wasting hours on potentially nothing.

Stan added a subscriber: smiley.Oct 15 2019, 4:58 PM
In D2377#99209, @smiley wrote:

Since apparently I had commented on this before, I have a feeling this would not quite work the way it should in certain cases.

Then again, its just a feeling. Consider that before wasting hours on potentially nothing.

Thanks for the fair warning, and the time you took to express it :)

Stan planned changes to this revision.Mar 15 2020, 5:18 PM

This should be updated.

Stan updated this revision to Diff 12936.Mon, Jul 27, 12:46 PM

Rebase.
Fix logic.
Add support for max length with paste
Remove casts.

The default case l324 seems useless but I'm not sure. I have not been able to enter it

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2789/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1157/display/redirect

Stan updated this revision to Diff 12991.Sat, Aug 1, 2:48 PM

Fix Unix* builds

Vulcan added a comment.Sat, Aug 1, 2:55 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2829/display/redirect

smiley added a comment.Sat, Aug 1, 4:59 PM

Thanks for the fair warning, and the time you took to express it :)

Actually, no. That was a snarky at best, counter productive at worst warning.

Regardless, your refactor fixed that bug.

wraitii requested changes to this revision.Sat, Aug 1, 5:18 PM
wraitii added a subscriber: wraitii.

You're missing the case where the user-autocompletes a pseudo, or any JS modifying "caption" in fact. You probably need to add a final check in UpdateText() or in HandleMessage.

Otherwise seems to work.

See D2629 for a bug where text-too-long crashes.

This revision now requires changes to proceed.Sat, Aug 1, 5:18 PM
Stan updated this revision to Diff 12995.Sat, Aug 1, 6:04 PM

Broken attempt @wraitii can you help me

Vulcan added a comment.Sat, Aug 1, 6:11 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2832/display/redirect

Stan updated this revision to Diff 13008.Sun, Aug 2, 3:22 PM

Fix curpos

Stan updated this revision to Diff 13009.Sun, Aug 2, 3:26 PM

Don't crop non maxlength inputs

Vulcan added a comment.Sun, Aug 2, 3:32 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2842/display/redirect

Vulcan added a comment.Sun, Aug 2, 3:37 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2843/display/redirect

wraitii accepted this revision.Mon, Aug 3, 1:46 PM

Seems to work fine now.

This revision is now accepted and ready to land.Mon, Aug 3, 1:46 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Mon, Aug 3, 2:39 PM