Closed Bug 332211 Opened 19 years ago Closed 19 years ago

Problem updating UI with external instances

Categories

(Core Graveyard :: XForms, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: allan, Assigned: aaronr)

References

()

Details

(Keywords: fixed1.8.0.5, fixed1.8.1)

Attachments

(4 files)

There seems to be a problem with updating the UI when you use external instances. This might be a regression from bug 305060.
Attached file External instance file (deleted) —
Attached file Testcase w/external instance (deleted) —
Attached file Testcase w/inline instance (deleted) —
Priority: -- → P1
Blocks: 331209
Ok, during initialization we call model::rebuild during FinishConstruction. Rebuild will set mNeedsRefresh based on the test for mDocumentLoaded. So the result is if we call rebuild after mDocumentLoaded is set, we'll refresh every control in the model. Looks like we expect mDocumentLoaded to be PR_FALSE during initialization, and PR_TRUE after initialization. However, mDocumentLoaded could be PR_TRUE if the external instance load doesn't complete until after the xforms document receives DOMContentLoaded. In this case we'll set mNeedsRefresh. Thus, the next time the model needs to refresh, it will refresh every control it contains rather than just the ones that depend on the changed node. In the failing testcase this means that after we select an item from the itemset, we then refresh all the controls, which will cause the itemset to purge all of its items from the document and clone new copies. So now the item that select1 thinks is currently selected is no longer in the DOM which will cause problems when the select1 goes to refresh.
Status: NEW → ASSIGNED
Attached patch first attempt (deleted) — Splinter Review
Well, the fix I propose is to us mReadyHandled instead of mDocumentLoaded. I'd argue that the form author shouldn't expect that monkeying with and then rebuilding the instance should work before xforms-ready. And certainly not before xforms-model-construct-done. There is a gap in between construct done and ready where we could possibly honor it. But in that case we should probably only refresh the controls that could bind and not refresh those that may still be on the deferredbindlist. But I'll float the mReadyHandled fix to Allan first to see what he thinks before going through the other complications.
Attachment #220203 - Flags: review?(allan)
Comment on attachment 220203 [details] [diff] [review] first attempt (In reply to comment #5) > Well, the fix I propose is to us mReadyHandled instead of mDocumentLoaded. I'd > argue that the form author shouldn't expect that monkeying with and then > rebuilding the instance should work before xforms-ready. And certainly not > before xforms-model-construct-done. There is a gap in between construct done > and ready where we could possibly honor it. But in that case we should > probably only refresh the controls that could bind and not refresh those that > may still be on the deferredbindlist. But I'll float the mReadyHandled fix to > Allan first to see what he thinks before going through the other complications. Hmm, it should not. But even if it does, something else is wrong which should be fixed, because your logic seems 100% correct. r=me
Attachment #220203 - Flags: review?(allan) → review+
Attachment #220203 - Flags: review?(Olli.Pettay)
Attachment #220203 - Flags: review?(Olli.Pettay) → review+
checked into trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Keywords: fixed1.8.1
Keywords: fixed1.8.0.5
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: