Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix regular expression matching date patterns in date/datetime <select/>-based helpers, allowing support for other locales #184

Merged
merged 2 commits into from
Aug 1, 2022
Merged

Fix regular expression matching date patterns in date/datetime <select/>-based helpers, allowing support for other locales #184

merged 2 commits into from
Aug 1, 2022

Conversation

mimmi20
Copy link
Contributor

@mimmi20 mimmi20 commented Jul 29, 2022

This will escape some meta characters.

Fixes #160

@mimmi20 mimmi20 marked this pull request as ready for review July 29, 2022 19:42
Ocramius
Ocramius requested changes Jul 30, 2022
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few things to adjust:

  1. commit message does not explain what's going on - some more clarity needed there
  2. test case does not make the issue clear: if this test failed, I wouldn't understand why, nor what it represents

@Ocramius Ocramius added the Bug Something isn't working label Jul 30, 2022
@Ocramius Ocramius modified the milestones: 3.5.0, 3.4.1 Jul 30, 2022
@Ocramius Ocramius changed the base branch from 3.5.x to 3.4.x July 30, 2022 09:32
@mimmi20 mimmi20 changed the title escape character escape regex-meta-character in AbstractFormDateSelect Jul 30, 2022
@mimmi20 mimmi20 requested a review from Ocramius July 30, 2022 12:49
mimmi20 and others added 2 commits July 30, 2022 21:31
@mimmi20
escape regex-meta-character in AbstractFormDateSelect and FormDateTim…
0372311 
…eSelect

Signed-off-by: Thomas Müller <[email protected]>
@Ocramius
Tests for #160 and #184: showing broken datetime picker rendering in …
faefb5b 
…`es_CL`+short formats

This change introduces verbose XML-alike-structure verification, which checks:

 1. that the HTML generated by `FormDateSelect` and `FormDateTimeSelect`
    matches the requested locale (we had zero tests verifying the generated HTML structure!)
 2. that the HTML generation actually works (fails on `3.4.x` / 0ca4165 )

The underlying issue was that the date format was being parsed by a bogus regular expression (fixed
in previous commit), which in turn led to a PHP warning being raised, as well as HTML rendering being
completely broken.

This test validates the work done by @mimmi20 in #184, and also highlights how antiquate
these HTML select elements are (please use `<input type="datetime-local"/>` or similar instead).

Signed-off-by: Marco Pivetta <[email protected]>
@Ocramius Ocramius self-assigned this Aug 1, 2022
@Ocramius Ocramius merged commit cd3f9d3 into laminas:3.4.x Aug 1, 2022
@Ocramius Ocramius changed the title escape regex-meta-character in AbstractFormDateSelect Fix regular expression matching date patterns in date/datetime <select/>-based helpers, allowing support for other locales Aug 1, 2022
@github-actions github-actions bot mentioned this pull request Aug 1, 2022
Merged
@Ocramius Ocramius mentioned this pull request Aug 1, 2022
Closed
@mimmi20 mimmi20 deleted the issue-160 branch August 1, 2022 14:41
@Slamdunk Slamdunk mentioned this pull request Oct 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius requested changes

@Xerkus Xerkus Xerkus left review comments

Assignees

@Ocramius Ocramius

Labels
Bug Something isn't working
Projects
None yet
Milestone
3.4.1
Development

Successfully merging this pull request may close these issues.

Unescaped regex-meta-character in AbstractFormDateSelect
3 participants
@mimmi20 @Ocramius @Xerkus