label failing cross-domain check should trigger xforms-link-error

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
11 years ago
11 months ago

People

(Reporter: Allan Beaufour, Assigned: Allan Beaufour)

Tracking

({fixed1.8.0.5, fixed1.8.1})

Trunk
fixed1.8.0.5, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

1.12 KB, patch
smaug
: review+
Doron Rosenberg (IBM)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
"Dispatched as an indication of: a failure in link traversal of a linking attribute, in a situation not critical to form processing."
[http://www.w3.org/TR/2006/REC-xforms-20060314/slice4.html#evt-linkError]

Imho that means we should trigger it for failing cross-domain checks too. One could argue that it could be "critical to form processing", but without this there is no way the form author can find out. With the event, an error can at least be displayed to the user.
(Assignee)

Comment 1

11 years ago
Created attachment 222590 [details] [diff] [review]
Patch
Attachment #222590 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED

Comment 2

11 years ago
Comment on attachment 222590 [details] [diff] [review]
Patch

Makes sense
Attachment #222590 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Updated

11 years ago
Attachment #222590 - Flags: review?(doronr)

Comment 3

11 years ago
My issue with this would be compatibility.  We'd be generating an error in situations when other processors wouldn't.  And in reality, since we don't (and may never for 1.0, it seems) support context info on events, the event notification is practically useless.  The form author can't determine which control is linking to the problematic link or even what the problematic link is so it isn't like any error-recovery handling could really take place.  I'd be more in favor of a mozilla specific 'fatal error' that would trigger the fatal error dialog, which points the user to the JS console to see which link failed.  It would be easy to see in testing without the form author having to code his own message that he would eventually have to remove when they roll out the form.  

But I'm not completely against the idea.  It is a logical interpretation of the spec.  If we are going to take this approach, then I would think that it should be consistent across all of the controls and not just limited to label.  Help, hint, alert, load, and message all generate a xforms-link-error in the same scenario that label does, so if those link traversals fail due to cross-domain checks, then we should send the xforms-link-error then, too.

Updated

11 years ago
Attachment #222590 - Flags: review?(doronr) → review+
(Assignee)

Comment 4

11 years ago
(In reply to comment #3)
> My issue with this would be compatibility.  We'd be generating an error in
> situations when other processors wouldn't.  And in reality, since we don't (and
> may never for 1.0, it seems) support context info on events, the event
> notification is practically useless.  The form author can't determine which
> control is linking to the problematic link or even what the problematic link is
> so it isn't like any error-recovery handling could really take place.  I'd be
> more in favor of a mozilla specific 'fatal error' that would trigger the fatal
> error dialog, which points the user to the JS console to see which link failed.
>  It would be easy to see in testing without the form author having to code his
> own message that he would eventually have to remove when they roll out the
> form.  

<xf:message ev:event="xforms-link-error">
A control failed to retrieve necessary content. Please make sure that XXX, and YYY.
</xf:message>

I would like to include that as a form author. I personally hate pop-ups so, I would probably do a <xf:toggle ev:event="xforms-link-error" case="link-error"/>. I could even switch to a case where controls are using an entirely different model, making the form work in "offline mode" if there's network errors. Just a few quick ideas :-)

> But I'm not completely against the idea.  It is a logical interpretation of the
> spec.  If we are going to take this approach, then I would think that it should
> be consistent across all of the controls and not just limited to label.  Help,
> hint, alert, load, and message all generate a xforms-link-error in the same
> scenario that label does, so if those link traversals fail due to cross-domain
> checks, then we should send the xforms-link-error then, too.

That's a good idea. I'll do a follow-up bug for that.
(Assignee)

Comment 5

11 years ago
Fixed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Comment 6

11 years ago
(In reply to comment #4)
> (In reply to comment #3)
> > But I'm not completely against the idea.  It is a logical interpretation of the
> > spec.  If we are going to take this approach, then I would think that it should
> > be consistent across all of the controls and not just limited to label.  Help,
> > hint, alert, load, and message all generate a xforms-link-error in the same
> > scenario that label does, so if those link traversals fail due to cross-domain
> > checks, then we should send the xforms-link-error then, too.
> 
> That's a good idea. I'll do a follow-up bug for that.

Bug 338788
Blocks: 338788
(Assignee)

Comment 7

11 years ago
Created attachment 222847 [details] [diff] [review]
Patch

Also dispatch xforms-link-error on security errors.

If I understand message correct (?), then we were missing a mStopType setting on redirects, which I have now added.
Attachment #222847 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 8

11 years ago
Comment on attachment 222847 [details] [diff] [review]
Patch

Ooops. Wrong bug.
Attachment #222847 - Attachment is obsolete: true
Attachment #222847 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.0.5
(Assignee)

Updated

11 years ago
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.