repeat-attrs on table causing complex content binding error

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
10 years ago
11 months ago

People

(Reporter: Steve Speicher, Assigned: aaronr)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
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
(Reporter)

Comment 1

10 years ago
Created attachment 261940 [details]
test case
(Assignee)

Comment 2

10 years ago
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.
Assignee: xforms → aaronr
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

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

made fix using check for repeat attrs, so no more error message popping up.
Attachment #262051 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 4

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

removing request.  I screwed something up.
Attachment #262051 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 5

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

Comment 6

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

Overrode UpdateRepeatState on repeat to figure out if it is an attr based repeat.  Beats moving mElement into stub element.
Attachment #262051 - Attachment is obsolete: true
Attachment #262094 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 7

10 years ago
Created attachment 262096 [details] [diff] [review]
patch for 1.8 and 1.8.0 branches
(Assignee)

Updated

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

Comment 8

10 years ago
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.
Attachment #262096 - Attachment is obsolete: true

Comment 9

10 years ago
Comment on attachment 262094 [details] [diff] [review]
patch2


>+  /**
>+   * 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.
Attachment #262094 - Flags: review?(surkov.alexander) → review+

Updated

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

Comment 10

10 years ago
Created attachment 262186 [details] [diff] [review]
patch for checkin

fixes surkov's nits, ready for checkin once the tree goes green
Attachment #262094 - Attachment is obsolete: true
(Assignee)

Comment 11

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

Comment 12

10 years ago
checked into 1.8 and 1.8.0 branches
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.