Last Comment Bug 377874 - repeat-attrs on table causing complex content binding error
: repeat-attrs on table causing complex content binding error
: fixed1.8.0.12, fixed1.8.1.4
Product: Core
Classification: Components
Component: XForms (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: aaronr
: Stephen Pride
Depends on:
  Show dependency treegraph
Reported: 2007-04-18 06:38 PDT by Steve Speicher
Modified: 2007-04-24 19:15 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

test case (1.53 KB, application/xhtml+xml)
2007-04-18 06:39 PDT, Steve Speicher
no flags Details
patch (1.81 KB, patch)
2007-04-18 16:02 PDT, aaronr
no flags Details | Diff | Review
patch2 (5.48 KB, patch)
2007-04-19 00:18 PDT, aaronr
bugs: review+
surkov.alexander: review+
Details | Diff | Review
patch for 1.8 and 1.8.0 branches (12.09 KB, patch)
2007-04-19 00:42 PDT, aaronr
no flags Details | Diff | Review
patch2 for 1.8 and 1.8.0 branches (8.35 KB, patch)
2007-04-19 00:55 PDT, aaronr
no flags Details | Diff | Review
patch for checkin (5.53 KB, patch)
2007-04-19 15:40 PDT, aaronr
no flags Details | Diff | Review

Description Steve Speicher 2007-04-18 06:38:30 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070416 Minefield/3.0a4pre
Build Identifier: 

This fails non-normative W3C test suite case 9.3.2.a

Reproducible: Always
Comment 1 Steve Speicher 2007-04-18 06:39:13 PDT
Created attachment 261940 [details]
test case
Comment 2 aaronr 2007-04-18 15:35:55 PDT
problem here is the initial output from the document doesn't live under a range since the range is introduced via anonymous content.  So when we run it through UpdateRepeatState, it is coming back as eType_NotApplicable.  Will have to change the logic there to look for xforms repeat attrs.  Well, at least the attr(s) that introduce repeats into anonymous content.
Comment 3 aaronr 2007-04-18 16:02:48 PDT
Created attachment 262051 [details] [diff] [review]

made fix using check for repeat attrs, so no more error message popping up.
Comment 4 aaronr 2007-04-18 16:20:14 PDT
Comment on attachment 262051 [details] [diff] [review]

removing request.  I screwed something up.
Comment 5 aaronr 2007-04-18 17:17:31 PDT
(In reply to comment #4)
> (From update of attachment 262051 [details] [diff] [review])
> removing request.  I screwed something up.

Crap.  If we detect repeat attrs on the parent, we need to know if the current element is a repeat or not.  If it is then it is the anonymous repeat that is contained by the element with repeat attrs and we need to give it eType_NotApplicable.  Otherwise it'll be eType_Template.  However, at the stub element level we don't have access to mElement.  So looks like we are going to have to store mElement in stub element instead of in control stub, action, etc.  It is more overhead for every xforms element now, but not as much as having to move the repeat state and functions out of stub element.
Comment 6 aaronr 2007-04-19 00:18:37 PDT
Created attachment 262094 [details] [diff] [review]

Overrode UpdateRepeatState on repeat to figure out if it is an attr based repeat.  Beats moving mElement into stub element.
Comment 7 aaronr 2007-04-19 00:42:10 PDT
Created attachment 262096 [details] [diff] [review]
patch for 1.8 and 1.8.0 branches
Comment 8 aaronr 2007-04-19 00:55:23 PDT
Created attachment 262098 [details] [diff] [review]
patch2 for 1.8 and 1.8.0 branches

oops, forgot I had trace in the tree from another bug.
Comment 9 alexander :surkov 2007-04-19 03:52:46 PDT
Comment on attachment 262094 [details] [diff] [review]

>+  /**
>+   * Override of nsXFormsStubElement's function.  Needed to properly handle
>+   * the case where the repeat is generated by anonymous content.

nit: nice if you would mention here about attr-based repeat binding because it's not clear about what anon content you said.

>+    if (NS_SUCCEEDED(rv))
>+      // if repeat is attribute based, it won't go through the normal
>+      // attribute change processing, so should set the mIsAttributeBased
>+      // variable here
>+      mIsAttributeBased = isAttributeBased;
>+      if (isAttributeBased) {
>+        repeatState = eType_NotApplicable;
>+        SetRepeatState(repeatState);
>+        return repeatState;
>+      }
>+  }

nit: either line up properly if (isAttributeBased) or add brackets for top if statement.
Comment 10 aaronr 2007-04-19 15:40:05 PDT
Created attachment 262186 [details] [diff] [review]
patch for checkin

fixes surkov's nits, ready for checkin once the tree goes green
Comment 11 aaronr 2007-04-19 19:09:18 PDT
checked into trunk
Comment 12 aaronr 2007-04-24 19:15:55 PDT
checked into 1.8 and 1.8.0 branches

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