modbvalue onchange() regression
It looks like 0b3fa98 causes the onchange() function to not be called for an modbvalue in certain situations. Specifically, when the user uses an editable modbvalue to change the value in the ODB.
The current flow is:
- User clicks modbvalue “link” and edits the value
ODBFinishInlineEdit
updates the value in the ODB, then callsmie_back_to_link
mie_back_to_link
reads the new value from the ODB and updates the element’s value attribute- Eventually
mhttpd_scan
reads the new value from ODB and calls onchange() if the new value differs from the element’s value attribute
The problem is that by updating the value attribute in step 3, we fail the “has the value changed?” test in step 4, and so don’t call onchange(). If I comment out the line added in 0b3fa98 then the onchange behaves as I expect.
Do you remember what bug you were fixing in that commit? Was it to do with cases where we’re calling mie_back_to_link
without updating the ODB value (e.g. because the user cancelled the change or validation failed)? If so, we might need to add another argument to mie_back_to_link
to state whether to update the value attribute or not.
Comments (3)
-
-
reporter - changed status to resolved
Fix issue
#364. Ensure onchange() is called for modbvalue in all circumstances.→ <<cset c24e91b898bd>>
-
reporter Done. Fixed in c24e91b
- Log in to comment
Indeed I should be more explicit with the changelog documentation, “fixed bug” does not tell you anything. Sorry for that. I actually had to remind myself what was the issue. It’s the following: If you change a value over a slow link (you can force that by adding artificial delays in mhttpd at the right place), then you see a strange effect. You change a modbvalu via clicking at it, and after you enter you see the OLD value. Only when the new value has propagated to the ODB and returned back, you see the new value. I though my quick fix would have been an easy solution, but as you tell me it prefents the onchange() being called. The obvious thing would be to call the onchange() manually like
Unfortunately I can’t test this right now, but please give it a try and report back here.