The default bug view has changed. See this FAQ.

Problem updating UI with external instances

RESOLVED FIXED

Status

Core Graveyard
XForms
P1
normal
RESOLVED FIXED
11 years ago
9 months ago

People

(Reporter: Allan Beaufour, Assigned: aaronr)

Tracking

({fixed1.8.0.5, fixed1.8.1})

Trunk
fixed1.8.0.5, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
There seems to be a problem with updating the UI when you use external instances. This might be a regression from bug 305060.
(Reporter)

Comment 1

11 years ago
Created attachment 216734 [details]
External instance file
(Reporter)

Comment 2

11 years ago
Created attachment 216735 [details]
Testcase w/external instance
(Reporter)

Comment 3

11 years ago
Created attachment 216736 [details]
Testcase w/inline instance
(Reporter)

Updated

11 years ago
Priority: -- → P1
(Reporter)

Updated

11 years ago
Blocks: 331209
(Assignee)

Comment 4

11 years ago
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.
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

11 years ago
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.
Attachment #220203 - Flags: review?(allan)
(Reporter)

Comment 6

11 years ago
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+
(Assignee)

Updated

11 years ago
Attachment #220203 - Flags: review?(Olli.Pettay)
Attachment #220203 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 7

11 years ago
checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Reporter)

Updated

11 years ago
Keywords: fixed1.8.1
(Reporter)

Updated

11 years ago
Keywords: fixed1.8.0.5
(Reporter)

Updated

11 years ago
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.