Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349957 - [client] Clean up the orion.editor.Editor API
Summary: [client] Clean up the orion.editor.Editor API
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Editor (show other bugs)
Version: 0.2   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 0.4 M1   Edit
Assignee: Silenio Quarti CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-21 11:07 EDT by Mark Macdonald CLA
Modified: 2011-12-01 17:00 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Macdonald CLA 2011-06-21 11:07:03 EDT
The Editor API should be more consumable.

In 0.2 clients are expected to use dojo.connect() to be notified about "events" like onInputChange, onDirtyChange. This makes sense if you're familiar with dojo, but eventually dojo will be removed from the editor layer, so we need a different API. Should consider an addEventListener model like TextView.
Comment 1 Felipe Heidrich CLA 2011-07-07 16:51:27 EDT
Note that first EventTable has to be made public.
Second, we have some problems we never solved:

a) how to prevent event propagation

b) how custom events related to dom event (see Bug 334583), this impacts the design of a solution for (a)

For a), if you look at jquery, they added all possible methods for that:

event.preventDefault() 
event.isDefaultPrevented()
event.stopImmediatePropagation()
event.isImmediatePropagationStopped()
event.stopPropagation()
event.isPropagationStopped()

(see http://api.jquery.com/category/events/)
once upon the time all these methods were in the w3 spec (DOM-Level-3-Events)
but some of them have being removed since:
event.isDefaultPrevented() -> event.isDefaultPrevented
event.isPropagationStopped() removed
event.isImmediatePropagationStopped() removed
(see http://www.w3.org/TR/DOM-Level-3-Events/)

Besides that there are all the old (ie) methods like event.returnValue and event.cancelBubble.
Comment 2 Mark Macdonald CLA 2011-08-03 17:55:21 EDT
(In reply to comment #1)
Re: (b), in the editor case there's no clear relationship between custom (editor) events and DOM events. The editor events are synthesized, and are not necessarily caused by DOM events. They also seem to be more high-level than TextView events. So the concerns about bubbling, cancelling, and preventing the default behavior don't really apply...

With that in mind, I have a branch [1] that:
 1. Makes EventTable into a top-level class orion.textview.EventTable.
 2. Makes Editor implement the EventTarget interface [2].
 3. Makes events dispatched by Editor implement the Event interface [3], specifically CustomEvent [4].
    Any type-specific data of the event is stored in its "detail" field.
Anything related to propagation (eg. useCapture, preventDefault, etc), is ignored.

Keeping close to DOM event APIs seems like a good idea, but it does make Editor inconsistent with TextView wrt method signatures. Thoughts?

[1] http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?h=bug349957_event&id=b3fb7a7c172b4b4ed460c464d3ddfe633f8844a1
[2] http://www.w3.org/TR/DOM-Level-3-Events/#events-EventTarget
[3] http://www.w3.org/TR/DOM-Level-3-Events/#interface-Event
[4] http://www.w3.org/TR/DOM-Level-3-Events/#events-CustomEvent
Comment 3 Felipe Heidrich CLA 2011-08-04 15:52:07 EDT
I guess it makes sense to follow the w3c spec in this case.
That said, the EventTarget interface [2] does not offer any support for the context.
Comment 4 Felipe Heidrich CLA 2011-08-04 15:52:39 EDT
SSQ, what do you think ?
Comment 5 Mark Macdonald CLA 2011-08-30 15:50:11 EDT
Any more thoughts on this? I agree with Comment 3 that not having a "context" parameter is inconvenient, since it forces you to Function.bind.
Comment 6 Silenio Quarti CLA 2011-10-28 14:56:15 EDT
Fixed
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=9c558cea5253413d08c4e2a4d9ad0ea20009a87f

Separated EventTarget from the text view and made it a mixin. Changed all classes in the editor bundle to use it including the text models.

We also switch to use require js when available and we are not polluting the global scope anymore.

We should still investigate either the EventTarget should be common between the service registry and the editor.
Comment 7 Felipe Heidrich CLA 2011-10-28 15:34:20 EDT
Still need to investigate the adoption of the Event interface.
We will need 'something' in our event to indicate that the default handler should not be called.
Comment 8 Felipe Heidrich CLA 2011-11-10 15:24:24 EST
I reopened this bug by accident,