blink: clean up test comment for datetime-local year auto-advance [chromium/src : main]
Abhishek Sundriyal (Gerrit)
Abhishek Sundriyal added 2 comments![]( "Open in Gerrit")
updated the comment to reference the bug instead
keyDown('3'); // 01/01/[0203] (old bug: jumped here)Abhishek SundriyalRemove this comment - no need to document old bugs. Alternatively, you could change this comment to `See crbug.com/473454959`.
updated the comment to reference the bug instead
Related details
- Keishi Hattori
- Mason Freed
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abhishek Sundriyal (Gerrit)
Abhishek Sundriyal added 3 comments![]( "Open in Gerrit")
Abhishek SundriyalNote: I have signed the Google CLA. AUTHORS sync may still be pending,
so I uploaded using --bypass-hooks.
Done
Name and email added in authors file
Name and email added in authors file & updated the comment to reference the bug instead
keyDown('3'); // 01/01/[0203] (old bug: jumped here)Abhishek SundriyalRemove this comment - no need to document old bugs. Alternatively, you could change this comment to `See crbug.com/473454959`.
updated the comment to reference the bug instead
Done
Related details
- Keishi Hattori
- Mason Freed
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abhishek Sundriyal (Gerrit)
Message from Abhishek Sundriyal![]( "Open in Gerrit")
- Abhishek Sundriyal
- Keishi Hattori
- Mason Freed
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 4 comments![]( "Open in Gerrit")
blink: clean up test comment for datetime-local year auto-advanceWhat test comment? This looks like its changing the behavior to fix the bug
Abhishek Sundriyal <abhishe...@gmail.com>This doesn't look alphabetically sorted with the other names
bool is_short_field = maximum_length <= 2;This change is probably fine, but the old code is one of various things which tries to prevent users from entering invalid dates. Want to add a RuntimeEnabledFeature flag to control this behavior in case we have to revert this fix? Here is a patch which does that: https://chromium-review.googlesource.com/c/chromium/src/+/7045938
Related details
- Abhishek Sundriyal
- Keishi Hattori
- Mason Freed
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 1 comment![]( "Open in Gerrit")
bool is_short_field = maximum_length <= 2;This change is probably fine, but the old code is one of various things which tries to prevent users from entering invalid dates. Want to add a RuntimeEnabledFeature flag to control this behavior in case we have to revert this fix? Here is a patch which does that: https://chromium-review.googlesource.com/c/chromium/src/+/7045938
- Abhishek Sundriyal
- Keishi Hattori
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keishi Hattori (Gerrit)
Keishi Hattori voted Commit-Queue+1![]( "Open in Gerrit")
- Abhishek Sundriyal
- Xiaocheng Hu
Abhishek Sundriyal (Gerrit)
Abhishek Sundriyal added 5 comments![]( "Open in Gerrit")
please let me know if more changes are needed
blink: clean up test comment for datetime-local year auto-advanceWhat test comment? This looks like its changing the behavior to fix the bug
datetimelocal-mutiple-fields-keyboard-events.html
line 263 to 272
should i remove this test?
Want to add a `Bug: 473454959` footer here?
added Bug: 473454959 footer
This doesn't look alphabetically sorted with the other names
sorted alphabetically
Mason FreedThis change is probably fine, but the old code is one of various things which tries to prevent users from entering invalid dates. Want to add a RuntimeEnabledFeature flag to control this behavior in case we have to revert this fix? Here is a patch which does that: https://chromium-review.googlesource.com/c/chromium/src/+/7045938
+1
- Joey Arhar
- Keishi Hattori
- Mason Freed
- Xiaocheng Hu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 5 comments![]( "Open in Gerrit")
blink: clean up test comment for datetime-local year auto-advanceAbhishek SundriyalWhat test comment? This looks like its changing the behavior to fix the bug
datetimelocal-mutiple-fields-keyboard-events.html
line 263 to 272
should i remove this test?
No, the test is fine. I just think that the commit message should be changed since the phrasing makes it sound like there are no behavior changes, only changes to tests.
How about this as a commit message?
```
Prevent date input auto-advance for invalid year
Bug: 473454959
Change-Id: Ifeb83caf6983a9fa8bafefa38bf3a0018484e1c3
```
Abhishek SundriyalWant to add a `Bug: 473454959` footer here?
added Bug: 473454959 footer
I don't see that in the footer
Having a newline in between the license comment and the first include is how all our files are formatted. want to add it back?
status: "experimental",you can set this to stable, we can change it back to experimental if we run into issues with users.
<meta name="runtime-enabled-features"want to remove this indentation?
Related details
- Abhishek Sundriyal