Closed Bug 1352799 Opened 6 years ago Closed 6 years ago

dynamic maxlength in input form didn't work

Categories

(Core :: DOM: Editor, defect, P3)

Product:
Core
Component:
DOM: Editor
Version:
52 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox52 - ---
firefox55 --- fixed

People

(Reporter: h_enzi, Assigned: m_kato)

References

(
URL
)

Details

(Keywords: dev-doc-complete, testcase)

Attachments

(2 files)

Bug 1352799 - Always set maxlength on initializing editor.
59 bytes, text/x-review-board-request
masayuki
: review+
Details
Bug 1352799 - Part 2. Add test for changing maxlength dynamically.
59 bytes, text/x-review-board-request
masayuki
: review+
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170323105023

Steps to reproduce:

i changed the value of input's maxlength dynamically using javascript


Actual results:

the value of maxlength did changed, for example from maxlength='2' to maxlength='4'
(i checked it via inspect element)



Expected results:

i can't type more than 2 character when the maxlength value is changed to 4
forgot to mention, it works when i changed the value of maxlength to 0, then change it again using setTimeout
Could you attach a minimal testcase, please.
Flags: needinfo?(h_enzi)
Keywords: testcase-wanted
[Tracking Requested - why for this release]:
Need more info with atleast minimum test case to reproduce futher h_enzi@yahoo.co.id?
[bugday-20170403]
tracking-firefox52: --- → ?
Need more information from the user to reproduce it further
[bugday-20170403]
Component: Untriaged → Editor
Product: Firefox → Core
Too late for 52, and it doesn't sound like this is in a state where it can be tracked for future releases.
tracking-firefox52: ? → -
I can confirm this one, please see:

https://jsfiddle.net/n44cbhgd/1/

steps to reproduce:

- click b1
- input: "12" into the textbox
- click b2
- click b3
- input something into the textbox, it should be 4 char max but we can't input more than 2

thanks
sorry the URL should be:

https://jsfiddle.net/n44cbhgd/1/
sorry the URL should be:

https://jsfiddle.net/n44cbhgd/2/
Old FF33 has the bug too.
URL: https://jsfiddle.net/n44cbhgd/2/
Keywords: testcase-wanted → testcase
Flags: needinfo?(h_enzi)
nsTextControlFrame sets SetMaxTextLLength to editor, but due to display = none, RestyleManager doesn't call nsTextControlFrame::AttributeChanged.  So we should move calling SetMaxTextLength to HTMLInputElement or nsTextEditorState
Assignee: nobody → m_kato
Priority: -- → P3
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8860841 [details]
Bug 1352799 - Part 2. Add test for changing maxlength dynamically.

https://reviewboard.mozilla.org/r/132834/#review135692

::: editor/libeditor/tests/test_bug1352799.html:60
(Diff revision 1)
> +    window.setTimeout(() => {
> +      input.removeAttribute('maxlength');
> +      inputcontainer.style.display = 'block';
> +
> +      input.focus();
> +
> +      synthesizeKey('1', {});
> +      synthesizeKey('2', {});
> +      synthesizeKey('3', {});
> +      synthesizeKey('4', {});
> +      synthesizeKey('5', {});
> +      synthesizeKey('6', {});
> +      synthesizeKey('7', {});
> +      synthesizeKey('8', {});
> +
> +      is(input.value, '12345678', 'value should be 12345678 without maxlength');
> +
> +      SimpleTest.finish();

Could you add a test which sets smaller number to maxlength. Then, I guess that the value should be truncated. If you'd need to change the behavior for compatibility with other browsers, it'd be okay to use todo_is().
Attachment #8860841 - Flags: review?(masayuki) → review+
Comment on attachment 8860840 [details]
Bug 1352799 - Always set maxlength on initializing editor.

https://reviewboard.mozilla.org/r/132832/#review135694
Attachment #8860840 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/45f61d7420ec
Always set maxlength on initializing editor. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/d309a5a0a165
Part 2. Add test for changing maxlength dynamically. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/45f61d7420ec
https://hg.mozilla.org/mozilla-central/rev/d309a5a0a165
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
*Might* be better to be in "Firefox 55 for Developers", but up to writers.
Keywords: dev-doc-needed
OS: Unspecified → All
Hardware: Unspecified → All
I've added a note to the Fx55 release notes to cover this; see the bottom of:

https://developer.mozilla.org/en-US/Firefox/Releases/55#DOM_HTML_DOM

Let me know if you think the wording is OK. Thanks!
Keywords: dev-doc-needed → dev-doc-complete
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #21)
> I've added a note to the Fx55 release notes to cover this; see the bottom of:
> 
> https://developer.mozilla.org/en-US/Firefox/Releases/55#DOM_HTML_DOM
> 
> Let me know if you think the wording is OK. Thanks!

Thanks, looks fine to me.
You need to log in before you can comment on or make changes to this bug.