copyItem doesn't generate xforms-value-changed

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
11 years ago
9 months ago

People

(Reporter: aaronr, Assigned: aaronr)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

Trunk
x86
All
fixed1.8.0.12, fixed1.8.1.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

2.52 KB, application/xhtml+xml
Details
3.49 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
If a user selects a copyItem from a select or select1, a xforms-value-changed event doesn't not get generated.
(Assignee)

Comment 1

11 years ago
(In reply to comment #0)
> If a user selects a copyItem from a select or select1, a xforms-value-changed
> event doesn't not get generated.
> 

Should read 'doesn't get generated' Its my inner redneck trying to break out :-)
(Assignee)

Comment 2

11 years ago
Created attachment 245870 [details]
testcase
(Assignee)

Updated

10 years ago
Blocks: 353738
(Assignee)

Comment 3

10 years ago
One issue is that it shows as out of range and throws an out of range message to begin with.  That is because it is refreshing before the itemset is done creating all of its items.  Probably not related to the value-changed part of the bug, though.
(Assignee)

Comment 4

10 years ago
Created attachment 258850 [details] [diff] [review]
patch

Simple fix was to mark the node as changed during mdg's SetNodeContent, so that the xforms-value-changed event will be dispatched.  I also included fixes for two tinderbox build warnings that we currently have.

The issue of the select1 being initially marked as out of range will need to be fixed in the select/select1 bug -> bug 372197, since it is getting caused by the fact that we now create items due to mutation events.
Assignee: xforms → aaronr
Status: NEW → ASSIGNED
Attachment #258850 - Flags: review?(Olli.Pettay)

Updated

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

Updated

10 years ago
Attachment #258850 - Flags: review?(surkov.alexander)

Comment 5

10 years ago
Comment on attachment 258850 [details] [diff] [review]
patch


>-    nsresult rv;
>+    nsresult rv = NS_OK;
>     for (PRInt32 i=0; i<mPendingInlineSchemas.Count(); ++i) {
>       GetSchemaElementById(mElement, *mPendingInlineSchemas[i],
>                            getter_AddRefs(el));
>       if (!el) {
>         rv = NS_ERROR_UNEXPECTED;
>       } else {
>         if (!IsDuplicateSchema(el)) {
>           nsCOMPtr<nsISchema> schema;

If IsDuplicateSchema(el) == PR_TRUE then it will throw dublicateSchema error. Don't we need to 
have "schemaLoadError" error additionally (previously it was randomly I guess)? r=me with this answered and fixed if it's needed.
Attachment #258850 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 6

10 years ago
(In reply to comment #5)
> (From update of attachment 258850 [details] [diff] [review])
> 
> >-    nsresult rv;
> >+    nsresult rv = NS_OK;
> >     for (PRInt32 i=0; i<mPendingInlineSchemas.Count(); ++i) {
> >       GetSchemaElementById(mElement, *mPendingInlineSchemas[i],
> >                            getter_AddRefs(el));
> >       if (!el) {
> >         rv = NS_ERROR_UNEXPECTED;
> >       } else {
> >         if (!IsDuplicateSchema(el)) {
> >           nsCOMPtr<nsISchema> schema;
> 
> If IsDuplicateSchema(el) == PR_TRUE then it will throw dublicateSchema error.
> Don't we need to 
> have "schemaLoadError" error additionally (previously it was randomly I guess)?
> r=me with this answered and fixed if it's needed.
> 

I don't believe so.  Having a duplicate schema defined isn't a fatal error, I don't think.  It isn't spelled out really well in the spec.  The spec just says that it shouldn't happen, not what should occur if it should happen.  We should just ignore the duplicate schema and continue on, I guess.  But of course this could lead to other problems like what if a type MIP is set with a value being a type from this ignored schema?  The other processors (Novell, X-Smiles and formsPlayer) don't seem to do anything at all.  Not even throw an error.  At least we do that much :-)

So I'll check this patch in as is.

Comment 7

10 years ago
It's fine with me. Though a little comment won't be disadvantage :).
(Assignee)

Comment 8

10 years ago
Created attachment 259006 [details] [diff] [review]
patch2

patch for checkin.  Added comment about duplicate schema.
Attachment #258850 - Attachment is obsolete: true
(Assignee)

Comment 9

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

Comment 10

10 years ago
checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16
Keywords: fixed1.8.0.12, fixed1.8.1.4
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.