Closed
Bug 278368
Opened 20 years ago
Closed 19 years ago
MDG fails to find all dependencies, breaks refreshing
Categories
(Core Graveyard :: XForms, defect)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: allan, Assigned: allan)
References
()
Details
Attachments
(2 files, 5 obsolete files)
3.48 KB,
application/xhtml+xml
|
Details | |
9.14 KB,
patch
|
aaronr
:
review+
|
Details | Diff | Splinter Review |
The patch in bug 265467 has a hack that disables the selective refreshing in nsXFormsModelElement::Revalidate().
Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•20 years ago
|
||
Assignee | ||
Comment 2•20 years ago
|
||
Assignee | ||
Comment 3•20 years ago
|
||
Added trigger with setvalues without using action
Attachment #175428 -
Attachment is obsolete: true
Assignee | ||
Comment 4•20 years ago
|
||
Simplified the patch a bit. There's a still a problem with output's with @value, but I'm getting there.
Attachment #175427 -
Attachment is obsolete: true
Assignee | ||
Comment 5•20 years ago
|
||
(In reply to comment #3) > Created an attachment (id=175430) [edit] > Testcase v2 > > Added trigger with setvalues without using action The reason for this is that I'm experiencing weird behaviour with deferred actions. Using the patch and triggering the setvalues inside the action-block, only updates some of the outputs. Refreshing the page and trying again may update some other outputs. Weird weird. At the moment I'm not sure where the bug is hiding.
Assignee | ||
Comment 6•19 years ago
|
||
This should take care of it. Only problem is that it's less effective, but apparently I cannot use the pointers.
Attachment #175431 -
Attachment is obsolete: true
Attachment #175821 -
Flags: review?(smaug)
Comment 7•19 years ago
|
||
Comment on attachment 175821 [details] [diff] [review] Patch v3 >+ nsCOMPtr<nsIDOM3Node> curChanged; >+ PRBool rebind = PR_FALSE; > PRBool refresh = PR_FALSE; > > for (PRInt32 j = 0; j < mChangedNodes.Count(); ++j) { >- curChanged = mChangedNodes[j]; >- >- if (curChanged == boundNode) { >- refresh = PR_TRUE; >- // We cannot break here, as we need to to check for any changed >- // dependencies >- } >- >- if (depPos == depCount) { >- continue; >- } >+ curChanged = do_QueryInterface(mChangedNodes[j]); Hmm, DOM3Node is a tearoff, so this creates quite many temporary objects. But what would be a better way to do this (?) >- if ( aNode->mEndIndex < 0 >- || aNode->mStartIndex >= aNode->mEndIndex >- || ((PRUint32) aNode->mEndIndex == mCurExprString->Length() && aNode->mStartIndex == 0)) { >+ if (aNode->mEndIndex < 0 || >+ aNode->mStartIndex >= aNode->mEndIndex) { nit, remove the newline after || r=me, though I don't know the nsXFormsXPathAnalyzer code too well. (The patch isn't quite up-to-date with trunk.)
Attachment #175821 -
Flags: review?(smaug) → review+
Assignee | ||
Comment 8•19 years ago
|
||
(In reply to comment #7) > (From update of attachment 175821 [details] [diff] [review] [edit]) > Hmm, DOM3Node is a tearoff, so this creates quite many temporary objects. > But what would be a better way to do this (?) Beats me.
Assignee | ||
Comment 9•19 years ago
|
||
(In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 175821 [details] [diff] [review] [edit] [edit]) > > Hmm, DOM3Node is a tearoff, so this creates quite many temporary objects. > > But what would be a better way to do this (?) > > Beats me. Fixing bug 235512 might make the previous method work... let's see.
Assignee | ||
Comment 10•19 years ago
|
||
Could you do the 2nd review Aaron?
Attachment #175821 -
Attachment is obsolete: true
Attachment #179839 -
Flags: review?(aaronr)
Comment 11•19 years ago
|
||
Comment on attachment 179839 [details] [diff] [review] updated to current CVS + nsAutoString depNodeName; + for (PRUint32 t = 0; t < depCount; ++t) { + nsCOMPtr<nsIDOMNode> tmpdep = deps->ObjectAt(t); + NS_ENSURE_STATE(tmpdep); I'm not a big fan of bailing in debug code when you don't bail in the regular code with the same condition. Could cause havoc if a customer sees one behavior in the field and we try to debug it and see a different behavior. + if (!refresh && boundNode) { + curChanged->IsSameNode(boundNode, &refresh); + if (refresh) + continue; } I'd like more comments in ::Revalidate. Especially what refresh and rebind are for and why the tests that you are making. F.x., why does 'refresh' get set to true but then you do a continue on to the next node?
Attachment #179839 -
Flags: review?(aaronr) → review-
Assignee | ||
Comment 12•19 years ago
|
||
Attachment #179839 -
Attachment is obsolete: true
Attachment #180030 -
Flags: review?(aaronr)
Comment 13•19 years ago
|
||
Comment on attachment 180030 [details] [diff] [review] Fixed Aaron's comments dandy! r=me
Attachment #180030 -
Flags: review?(aaronr) → review+
Assignee | ||
Comment 14•19 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 15•19 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050411 Firefox/1.0+ I am not sure at all, but I think this patch may have caused extension BugMeNot (http://roachfiend.com/archives/2005/02/07/bugmenot/) to make firefox crash. I know extensions aren't your responsibility, but I thought I'd let you know incase it's something you can use to fix stuff. The crashing is reproducable. Install BugMeNot, go to any webpage, and bam! a crash. TBID TB5003308K Apologies if this patch isn't anything to do with what I am seeing.
Assignee | ||
Comment 16•19 years ago
|
||
(In reply to comment #15) > I am not sure at all, but I think this patch may have caused extension BugMeNot > (http://roachfiend.com/archives/2005/02/07/bugmenot/) to make firefox crash. The patch was to 'extensions/xforms' only, so unless BugMeNot uses XForms (which I doubt) I'm quite sure it's not related.
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•