Closed
Bug 279957
Opened 20 years ago
Closed 20 years ago
Rebuild on model element not fully implemented
Categories
(Core Graveyard :: XForms, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: aaronr, Assigned: aaronr)
References
Details
Attachments
(1 file, 3 obsolete files)
21.80 KB,
patch
|
bryner
:
superreview+
|
Details | Diff | Splinter Review |
We need to get nsXFormsModelElement::Rebuild() to work correctly. Allan has already commented in the function the general things that need to happen. Really just need to run ::Bind on all the controls again so that the bindings are re-generated after the MDG is cleared. Or something along those lines.
Well, we don't have to worry about people modifying forms via JS for a while. I tried to make a testcase to see if my implementation of rebuild worked and I got the following JS exception: Error: uncaught exception: [Exception... "Cannot modify properties of a WrappedNative" nsresult: "0x80570034 (NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN)" location: "JS frame :: file:///C:/myprogs/xml/xforms/recalculate.xhtml :: changeValue :: line 29" data: no] when I tried to change the ref attribute on an output element.
Status: NEW → ASSIGNED
Worked the approach out with Allan. I started by implementing ::Bind on elements that inherit from nsXFormsControlStub and moved the determination of mBoundNode from ::Refresh to ::Bind since that is executed a lot less often. In model::Rebuild(), I clear the MDG, then I backed up mFormControls array, cleared it, then went through the old list and ran ::Bind on all of the controls in the list. This will re-initialize mBoundNode and also get the controls re-inserted into the model that they are now bound to.
Attachment #172873 -
Flags: review?(allan)
Comment 3•20 years ago
|
||
Comment on attachment 172873 [details] [diff] [review] first attempt at fix >+++ nsXFormsModelElement.cpp 30 Jan 2005 12:02:15 -0000 >+#include "nsXFormsControlStub.h" nope, see below. >@@ -418,20 +419,46 @@ nsXFormsModelElement::Rebuild() ... >+ nsXFormsControlStub* control = NS_STATIC_CAST(nsXFormsControlStub*, >+ (*oldFormList)[i]); What's up with nsXFormsControlStub? bind is a funtion on nsIXFormsControl. >- if (!mBoundNode) >- return NS_OK; >+ // Seems to be a timing problem where the first call to Bind() during >+ // document load could fail. We'll try one more time here. >+ if(!mBoundNode) { >+ Bind(); >+ NS_ENSURE_STATE(mBoundNode); >+ } Doesn't sound good. "Seems" should be investigated. Moreover the node binding is optional, so you are not guaranteed a mBoundNode. Bigger issues: 1) Isn't the Bind() function you are implementing quite the same for most of the controls. Check whether it's possible to move the implementation to nsXFormsControlStub. 2) You have to reattach the <bind> elements to the MDG also.
Attachment #172873 -
Flags: review?(allan) → review-
incorporated Allan's comments. checked out the timing problem. It exists for all controls and isn't a problem. Bind failing to find mBoundNode because control isn't parented by a nsIXFormsContextControl. Eventually the select element will be appended into the HTMLBodyElement and then Bind will work and mBoundNode will be set accordingly.
Attachment #172873 -
Attachment is obsolete: true
Attachment #172934 -
Flags: review?(allan)
Comment 5•20 years ago
|
||
Comment on attachment 172934 [details] [diff] [review] updated patch 1. You moved implementation to nsXFormsControlStub, which is fine but: 1.1. You have changed the semantics of Bind(), you always return NS_OK, you should check/return the return-value of ResetBoundNode()! 1.2. ResetBoundNode() == Bind(), except for: * <group> and <switch> which saves the modelID (that'll go away with landing of bug 280017). * <input> which checks its visual representaion.... which only need to be checked in Refresh(). So, how about making nsXFormsControlStub::Bind() { return ResetBoundNode(); }; and make a NOOP Bind() in nsXFormsContextContainer 2. You actually change the meaning of refresh on <switch>, but as far as I can it's ok. But *ahem* Refresh() and Process() could use a bit of pruning :) One last go, and you're there.
Attachment #172934 -
Flags: review?(allan) → review-
incorporated comments. Left Bind in those that need to store the model id (like switch element which Beaufour said would be fixed in a different bug). Also things like ContextControl and RepeatElement who don't use Bind for mBoundNode will have Bind stubbed out. Otherwise everyone who needs to have mBoundNode set will use ::Bind from nsXFormsControlStub (which is most of the cases).
Attachment #172934 -
Attachment is obsolete: true
Attachment #173000 -
Flags: review?(allan)
Comment 7•20 years ago
|
||
Comment on attachment 173000 [details] [diff] [review] next attempt >+++ nsXFormsGroupElement.cpp 31 Jan 2005 20:51:35 -0000 >+nsXFormsGroupElement::Bind() ... >+ nsCOMPtr<nsIModelElementPrivate> modelPriv = nsXFormsUtils::GetModel(mElement); >+ nsCOMPtr<nsIDOMElement> modelElement = do_QueryInterface(modelPriv); The modelElement is hiding in mModel, ready for action :) nsCOMPtr<nsIDOMElement> modelElement = do_QueryInterface(mModel); And thththththat's all folks :) With that, r=me Only problem is that this will probably mega-clash with bug 280017... :(
Attachment #173000 -
Flags: review?(allan) → review+
Attachment #173000 -
Attachment is obsolete: true
Attachment #173009 -
Flags: superreview?(bryner)
Updated•20 years ago
|
Attachment #173009 -
Flags: superreview?(bryner) → superreview+
Comment 9•20 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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
•