Last Comment Bug 332211 - Problem updating UI with external instances
: Problem updating UI with external instances
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
P1 normal with 1 vote (vote)
: ---
Assigned To: aaronr
: Stephen Pride
:
Mentors:
http://www.w3.org/TR/xforms/
Depends on:
Blocks: 331209
  Show dependency treegraph
 
Reported: 2006-03-30 03:15 PST by Allan Beaufour
Modified: 2016-07-15 14:46 PDT (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
External instance file (453 bytes, text/xml)
2006-03-30 03:16 PST, Allan Beaufour
no flags Details
Testcase w/external instance (1.10 KB, application/xhtml+xml)
2006-03-30 03:18 PST, Allan Beaufour
no flags Details
Testcase w/inline instance (1.57 KB, application/xhtml+xml)
2006-03-30 03:19 PST, Allan Beaufour
no flags Details
first attempt (1.05 KB, patch)
2006-04-28 16:36 PDT, aaronr
allan: review+
bugs: review+
Details | Diff | Splinter Review

Description User image Allan Beaufour 2006-03-30 03:15:48 PST
There seems to be a problem with updating the UI when you use external instances. This might be a regression from bug 305060.
Comment 1 User image Allan Beaufour 2006-03-30 03:16:38 PST
Created attachment 216734 [details]
External instance file
Comment 2 User image Allan Beaufour 2006-03-30 03:18:56 PST
Created attachment 216735 [details]
Testcase w/external instance
Comment 3 User image Allan Beaufour 2006-03-30 03:19:29 PST
Created attachment 216736 [details]
Testcase w/inline instance
Comment 4 User image aaronr 2006-04-28 15:23:55 PDT
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.
Comment 5 User image aaronr 2006-04-28 16:36:29 PDT
Created attachment 220203 [details] [diff] [review]
first attempt

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.
Comment 6 User image Allan Beaufour 2006-05-04 06:44:38 PDT
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
Comment 7 User image aaronr 2006-05-11 14:22:13 PDT
checked into trunk

Note You need to log in before you can comment on or make changes to this bug.