exposing Event.originalTarget causes compatibility problems

NEW
Unassigned

Status

()

defect
--
minor
13 years ago
9 months ago

People

(Reporter: geckobugs, Unassigned)

Tracking

(Depends on 1 bug)

Trunk
mozilla1.9alpha8
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.2 ?

Firefox Tracking Flags

(Not tracked)

Details

()

User-Agent:       Opera/9.00 (Windows NT 5.1; U; en)
Build Identifier: Opera/9.00 (Windows NT 5.1; U; en)

Mozilla exposes an event object property called originalTarget. Websites have started using this, the most high-profile one is KLM, and it causes interoperability problems for other UAs like Opera and Safari. Its usage here:

http://www.klm.com/travel/no_en/static/js/consolidated/_content.js

currently means site navigation is broken in Opera.

Please consider removing Event.originalTarget or try to find a way you can limit it to being exposed to XUL/web app content only, not regular web pages.

Reproducible: Always

Steps to Reproduce:
1. load KLM.com in Opera
2. choose country/language if requested
3. try navigation menu

Actual Results:  
Error console reports JS problem

Expected Results:  
Opened navigation menu
confirming. 
Status: UNCONFIRMED → NEW
Component: General → DOM: Level 0
Ever confirmed: true
OS: Windows XP → All
Product: Firefox → Core
Hardware: PC → All
Version: unspecified → Trunk
We should fix this. It would be enough to just throw an exception when the method is called from regular Web content, since that would make it unusable.
This would break the Xforms XBL implementation, looks like.

How about this:
-- make originalTarget (and explicitOriginalTarget) throw when called without chrome privileges.
-- expose originalTarget as mozOriginalTarget for use by untrusted code (XForms XBL bindings)
-- update XForms XBL bindings to suit
Do this for Gecko1.9/FF3.

Zbigniew, I can help you do this if you don't mind taking it on. It will save you time overall because you'll spend less time in forums :-)
(In reply to comment #3)
> Zbigniew, I can help you do this if you don't mind taking it on. It will save
> you time overall because you'll spend less time in forums :-)

Convinced. Taking. :)

I'll need some guidelines on how to check if the method has chrome privileges first.
Assignee: nobody → gandalf
Severity: normal → minor
Target Milestone: --- → mozilla1.9beta
I guess changing this would break also other web content which is using XBL, 
not only XForms extension.
And probably also some remote XUL and... 
(In reply to comment #5)
> I guess changing this would break also other web content which is using XBL, 
> not only XForms extension.

Any idea who might be doing that?

We could add mozOriginalTarget so those people can at least unbreak themselves.

> And probably also some remote XUL and... 

We could also allow access from untrusted XUL documents.
QA Contact: general → ian
Event.explicitOriginalTarget is also found in the wild.
starting work on this
Status: NEW → ASSIGNED
Inviting DOM owners and peers for the party.

Just to make sure everyone is happy with the approach.
Basing on IRC discussion with bz, we'll throw NS_ERROR_DOM_SECURITY_ERR when called without chrome privileges, add mozOriginalTarget, clean our code and evanglize klm.com to fix it.
A few thoughts:

1)  Throwing on simple property access is generally bad.  It'll break sites
    that try to enumerate properties and show their values.  We've run into
    this before. So it's probably better to return null and warn to the JS
    console or something if we're doing this.
2)  How will this change help?  Just by making it clear that these are
    Mozilla-specific extensions to the DOM?  (Not that I don't think that's a
    good goal in this case, but people shouldn't expect the KLM site to
    magically start working in Opera after this change... in fact, the most
    likely outcome is that it will stop working in Gecko and continue not
    working in Opera; the next-most-likely is that they will switch to
    mozOriginalTarget).
3)  Make sure to switch all C++ callers to mozOriginalTarget or whatever,
    since you can't trust security checks in those cases.
4)  Make sure you switch any XBL that might be used by remote XUL over to
    mozOriginalTarget (spinbuttons, colorpickers, dialogs, popups, textboxes,
    trees, tabbrowsers, etc).
5)  What's the plan for existing XBL that's out there, if any?
6)  What does the current XBL2 draft have to say about this?  Is it adding a
    property along these  lines to events?  If so, what property?
Do we know *why* KLM is using .originalTarget instead of just .target?
Depends on: 174397
I don't agree with the proposed solution in comment 10. If we rename this property people are just going to use the new name.

.originalTarget is an important part of the XBL1 language, exposing it is intentional.

XBL2 does sort of improve the situation here since there is no .originalTarget and generally hides all traces of XBL to an outside user. However the problem here is really that the site is using mozilla specific features that Opera don't implement, even if we changed one such feature there are tons of other that sites could (and do) use.

The problem here is not with mozilla, but rather with KLM not sticking to standards that are implemented cross browser. There is no way mozilla is simply going to stop implementing new features just because sites end up using them. What we do and will continue to do is to help other browers implement these features. That's how we move the web forward.

I'd say this bug should be closed as INVALID and a new bug should be filed on KLM or on Opera evangelism so that they could contact KLM.
(In reply to comment #13)
> I don't agree with the proposed solution in comment 10. If we rename this
> property people are just going to use the new name.

Maybe so, but it still matters. If they use mozOriginalTarget then at least we know they're intentionally using a Mozilla extension.
There is currently no standardized way of marking extensions to the DOM as vendor-specific. If we do this for originalTarget there are lots of other properties we should do it for as well, so I think that's the scope we need to think in.
None of this is standardized, or in this case even specified, so I don't find the "there's no standard for the better way, so we should do the non-standard and worse thing"; having the property on events when there's no XBL in play is a bad scene, and hurts interoperability on the web.  We should have removed all traces of XBL from content in 1.9 anyway (:P) and taken the compatibility burden on ourselves, rather than creating these problems for other browsers, for so SO little gain.  (The 5 sites in the world that use XBL with HTML.)  But if nothing else can we make it non-enumerable in 1.9?  Turns out that if you enumerate an event object right now you can get an exception, because .originalTarget points to something in chrome.

Can we make it return null if there's no XBL involved in the dispatch?  Can we rename to mozOriginalTarget here for 1.9?  It'll be an easy fix for people who really want to be dealing with originalTarget, and the job only gets harder the more releases tick by with it out there.  Gandalf: you game? :)

We've used mozDrawText on canvas, and it worked fine -- people are much less likely to use it and _not_ know that it's a Mozilla extension.  Putting things in mozWhatever and publishing a specification is a fine way to advance the web; we don't need to tromp on general namespaces with unspecified behaviour to move things forward.

(prototype.js just basically did this to Safari and Firefox by using document.getElementsByClassName, which then clashed with the standardized version having different behaviour.  Golden Rule says we should take steps to be good citizens ourselves.)
(In reply to comment #16)
> because .originalTarget points to something in chrome.
er, what?

> Can we make it return null if there's no XBL involved in the dispatch?
Chrome needs .originalTarget, AFAIK.
What we could and should do is to make .originalTarget return .target if
.originalTarget is in native anonymous content.

>  Can we
> rename to mozOriginalTarget here for 1.9?
And you want to fix lot's of extensions? and chrome?
From PPK I've gotten the report that if you view this page in Firefox 2 or 3b4:
http://www.quirksmode.org/js/events/blurfocus.html

check 'Show event properties' and click on the text input or textarea. Now FF tries to print out all property values and gives the error message:

  Error: uncaught exception: Permission denied to get property
  HTMLDivElement.nodeName

Which is, most likely, something located in the chrome.
No. That is the native anonymous HTML div element inside HTML input/textarea element.
Which is likely the .originalTarget for which the text input's event is being dispatched, no?
Flags: wanted1.9.2?
QA Contact: ian → general
Depends on: 662335
Assignee: gandalf → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.