modbvalue onchange() regression

Issue #364 resolved
Ben Smith created an issue

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 calls mie_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)

  1. Stefan Ritt

    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

    line 234:
    
    if (p.value !== value)
      this.onchange();
    p.value = value;
    

    Unfortunately I can’t test this right now, but please give it a try and report back here.

  2. Log in to comment