Bug 331287 - Add at least stubs for Mouse Track Listeners to swt/widgets/Control
Summary: Add at least stubs for Mouse Track Listeners to swt/widgets/Control
Status: REOPENED
Alias: None
Product: RAP
Classification: RT
Component: RWT (show other bugs)
Version: 1.4   Edit
Hardware: All All
: P3 enhancement with 3 votes (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 338666 355373 373631 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-11-28 15:18 EST by PSko CLA
Modified: 2014-11-25 06:49 EST (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description PSko CLA 2010-11-28 15:18:18 EST
Hi All,

The problem is that org/eclipse/swt/widgets/Control.java lacks definition of certain methods. For example Mouse Track Listener related. This causes No Such Method exception to be thrown in swt apps using these methods.

I know that it would be problematic to handle mouse tracking in RWT, however, I would at least add their methods so the transition of SWT Apps to RWT could be seamless. No code alterations would be necessary in existing code written for SWT. For example, there are some widgets on the net, that work great but they use Mouse Track Listeners to provide nifty tooltips etc. The problem is that No Such Method exceptions are thrown when ran on RWT. 
I would not want to alter these widgets etc... I think it is ok to define all necessary methods in the SWT classes in such way that swt apps can run seamless,
even if these methods do nothing.

The proposition is to add to org/eclipse/swt/widgets/Control.java the following empty method definitions (stubs):

   public void addMouseTrackListener(MouseTrackListener listener) {
   }
   
   public void addMouseMoveListener(MouseMoveListener listener) {
   }
   
   public void removeMouseTrackListener(MouseTrackListener listener) {
   }
   
   public void removeMouseMoveListener(MouseMoveListener listener) {
   }
Comment 1 Ivan Furnadjiev CLA 2010-11-29 15:28:38 EST
Hi Piotr, I'm sorry that I will disappoint you, but we have principle to not provide unimplemented (empty) methods in RAP. Thus, when programmer get a compile errors, he knows exactly what is available in RAP and what is not. If we do, what you suggest, we will end in a endless list with reported malfunctions.
Comment 2 PSko CLA 2010-11-29 15:52:23 EST
(In reply to comment #1)
> Hi Piotr, I'm sorry that I will disappoint you, but we have principle to not
> provide unimplemented (empty) methods in RAP. Thus, when programmer get a
> compile errors, he knows exactly what is available in RAP and what is not. If
> we do, what you suggest, we will end in a endless list with reported
> malfunctions.

Hi Ivan,

I understand you point of view, however, I disagree :) 
Why do you keep using SWT API if does not implement things from it? Why don't you just use your own RWT api? You're trying to implement SWT without keeping ~compatibility (sorry, cannot find a better wording right now). There's so much effort to keep it compatible with SWT but you don't implement interfaces correctly (I know, you don't use swt interfaces, but I read some time ago, that you took it under consideration, dunno how it ended though)   

As to compile errors - nope, nobody gets compile errors when using third-party libraries (for example SWT widgets). What do they get instead are serious exceptions, that cannot be handled without dirty hacks. 

My suggestions are:
- stub methods producing warning log when executed 
- java annotations to tell the programmer that she/he should not use certain method (@Deprecated, I know that it may mislead people, however javadoc could describe clearly why is it marked such way).


Regards,
Piotr
Comment 3 Ivan Furnadjiev CLA 2010-11-30 01:42:12 EST
Hi Piotr, thank you for your opinion. I think that hiding "serious exceptions" by implementing empty methods will not help in making third-party SWT widgets workable. With or without these exceptions, these widgets in most cases will not work as expected as the actual functionality provided by the empty methods is missing. Maybe, you have a third-party widget that mouse track/move listeners are not vital for their functionality, but for other - it will be vital. My suggestion will be to use the source version of this widgets and to try to eliminate the compile errors yourself.
Comment 4 Ralf Sternberg CLA 2010-11-30 04:49:39 EST
Hi Piotr,

I see your point. We've also discussed this approach to implement all missing API as noops and let the tooling generate errors. But this is something that we cannot do for MouseTrackListeners only. It's also some non-trivial work to come up with a decent tooling solution, I think @deprecated is not an option. So we won't do this anytime soon.

However, if you have a strong feeling towards this approach, please open a bug for the entire issue and explain some case where this approach would allow for something that is currently not possible.
Comment 5 Missing name Mising name CLA 2011-04-06 08:06:19 EDT
Hello,
I follow you echange and I have a question :
In RAP 1.4, I miss addMouseMoveListener, addMouseTrackListener ... on Composite class. So the SashLayout class can be used and my SashContainer, in my perspective, can't be resizeable.
Do you know a solution to avoid this problem ? a bypass ?

Thanks a lot.
Comment 6 Igor Novakovic CLA 2011-10-07 11:46:55 EDT
(In reply to comment #4)
> Hi Piotr,
> 
> I see your point. We've also discussed this approach to implement all missing
> API as noops and let the tooling generate errors. But this is something that we
> cannot do for MouseTrackListeners only. It's also some non-trivial work to come
> up with a decent tooling solution, I think @deprecated is not an option. So we
> won't do this anytime soon.
> 
> However, if you have a strong feeling towards this approach, please open a bug
> for the entire issue and explain some case where this approach would allow for
> something that is currently not possible.

Hi Ralf,

a couple a weeks ago we did a proof of concept for deploying BPEL designer (we use it for designing our workflows) on RAP 1.4.0 (http://wiki.eclipse.org/SMILA/BPEL_Designer#RAP_showcase). One of the issues we had was this problem. We at SMILA (http://www.eclipse.org/smila/) think that RAP has a big potential in SaaS environments and it would be great if RAP would implement all missing API as noops since in that case we would not have to patch the code (that much) but be only faced with the features not functioning - which in our case is absolutely acceptable.

Best regards
Igor
Comment 7 PSko CLA 2011-10-07 13:13:23 EDT
> RAP has a big potential in SaaS environments and it would be great if RAP would
> implement all missing API as noops since in that case we would not have to
> patch the code (that much) but be only faced with the features not functioning
> - which in our case is absolutely acceptable.
> 

Igor, I guess we're on the same side of the barricade :)

A bit of sarcasm: I guess RAP doesn't want be too popular :>

I have no time to fight for it, to me it is easier and faster to patch the RAP code than trying to pursue for the fix. But maybe when more and more people start to vote to implement missing methods as NOOPs then RAP PMs will reconsider this approach :)

Regards,
Piotr
Comment 8 Igor Novakovic CLA 2011-10-10 11:26:36 EDT
(In reply to comment #7)
> > RAP has a big potential in SaaS environments and it would be great if RAP would
> > implement all missing API as noops since in that case we would not have to
> > patch the code (that much) but be only faced with the features not functioning
> > - which in our case is absolutely acceptable.
> > 
> 
> Igor, I guess we're on the same side of the barricade :)
> 
> A bit of sarcasm: I guess RAP doesn't want be too popular :>
> 
> I have no time to fight for it, to me it is easier and faster to patch the RAP
> code than trying to pursue for the fix. But maybe when more and more people
> start to vote to implement missing methods as NOOPs then RAP PMs will
> reconsider this approach :)
> 
> Regards,
> Piotr

Thank you Piotr for (still) participating in this discussion.
I asked Tobias (who actually did for us the "BPEL designer on RAP" proof of concept) to open a new issue where we can discuss this issue on a global level. So please take some time to support us also when this new issue has been submitted. (Tobias will post the link to it in this issue.)

Cheers
Igor
Comment 9 Ralf Sternberg CLA 2011-10-10 16:51:55 EDT
Hi Igor, hi Piotr,

I'm sorry to hear the frustration in your comments. It's in no way our intention to build up barricades!

The decision not to provide non-functional API (except some cases where API is clearly marked as hint) is driven by our belief that if we did, we would lull users in a false sense of security. Their code would compile but some parts would just not work correctly. The problems may be even hard to trace. If this decision seems to cause more trouble than it prevents, we should discuss this again.

Would it help if we provided a patch fragment that adds those empty APIs?

But then again, while MouseMoveListeners are an obvious case (they would simply never be called) what should be returned by a method Text#getLineCount()? always 0? If some code uses this value to make decisions, programs could behave falsely and the cause for this misbehavior is hard to detect...

Apart from this discussion, I think we can begin to remove barricades by implementing MouseTrackListeners, (the original purpose of this bug) which should be doable.
Comment 10 Ralf Sternberg CLA 2011-10-10 17:08:41 EDT
*** Bug 338666 has been marked as a duplicate of this bug. ***
Comment 11 Ralf Sternberg CLA 2011-10-10 17:09:16 EDT
Reopened bug for the original request to provide MouseTrackListener.
Comment 12 Arian Storch CLA 2011-10-11 04:49:54 EDT
I would appreciate some minimal events like a mouse over/hover or mouse entered/left for components. 

I think this wouldn't create to much traffic in praxis.
Comment 13 Igor Novakovic CLA 2011-10-11 13:10:55 EDT
(In reply to comment #9)
> Hi Igor, hi Piotr,
> 
> I'm sorry to hear the frustration in your comments. It's in no way our
> intention to build up barricades!
That was also my assumption.
Thank you for reopening this bug and participating in the discussion.


> The decision not to provide non-functional API (except some cases where API is
> clearly marked as hint) is driven by our belief that if we did, we would lull
> users in a false sense of security. Their code would compile but some parts
> would just not work correctly. The problems may be even hard to trace. If this
> decision seems to cause more trouble than it prevents, we should discuss this
> again.
Yes, I am very aware of this problem.
But let's take a look at this from another point of view.
What if one application should run in both worlds: As a "normal" RCP application and as a SaaS service deployed on RAP? In that case (we want to do this with BPEL designer (http://www.eclipse.org/bpel/)) we (currently) have to patch the RAP itself and merge that patch to every new release of RAP without getting any extra functionality. So it is quite an effort we have to invest _just_ to be able to deploy our application on RAP.


> Would it help if we provided a patch fragment that adds those empty APIs?
IMHO that would be a good start.


> But then again, while MouseMoveListeners are an obvious case (they would simply
> never be called) what should be returned by a method Text#getLineCount()?
> always 0? If some code uses this value to make decisions, programs could behave
> falsely and the cause for this misbehavior is hard to detect...
IMO each application has to go to through its quality assurance cycle - on each platform it is supposed to be run. So those problems should occur early.

I think that the major issue here is how to inform the developer (of the deployed application) that some methods are not really implemented. I personally would start with the clear logging pattern. The warning log message like e.g. "Warning: This method has been implemented as NOOP." would completely suffice.


> Apart from this discussion, I think we can begin to remove barricades by
> implementing MouseTrackListeners, (the original purpose of this bug) which
> should be doable.
Thank you.

Cheers
Igor
Comment 14 PSko CLA 2011-10-11 14:07:18 EDT
(In reply to comment #13)
> (In reply to comment #9)
> > Hi Igor, hi Piotr,
> > 
> > I'm sorry to hear the frustration in your comments. It's in no way our
> > intention to build up barricades!

No, it is not a frustration - a little bit of sarcasm doesn't harm :)
I just don't understand why RWT is trying to follow SWT, without keeping its compatibility - that's all. I can imagine the reasons, but it is a blocker for people that want to switch from standalone to SaaS/web application. I've done done, almost without pain (by patching RWT code) and it works as a charm - that is cool - big thanks to developers - helluva good job btw. 
SWT has a lot of widgets, some are open, some not - and here comes the problem, sometimes you cannot patch the widget. Some people want have two versions, standalone and SaaS - this can be hard to maintain both versions as the RWT API differs from SWT one. 


> > Would it help if we provided a patch fragment that adds those empty APIs?
> IMHO that would be a good start.

I would propose to make a list of non-existing methods and decide what next. If most of them are not so important (like MoveMoveListeners) we can assume that it won't harm anyone. 

The code that is noop'ed I would mark as @deprecated (it is not very pretty solution, but I have no other idea) - this would inform developers (via their IDEs) that something is wrong and will read javadocs, then developer can decide whether the app can live with nooped implementation or not. (What's more, it might happen that the developer, knowing the reasons why it is nooped, one will send you a patch with some nifty implementation :)).

> I think that the major issue here is how to inform the developer (of the
> deployed application) that some methods are not really implemented. I
> personally would start with the clear logging pattern. The warning log message
> like e.g. "Warning: This method has been implemented as NOOP." would completely
> suffice.

I agree.

> 
> 
> > Apart from this discussion, I think we can begin to remove barricades by
> > implementing MouseTrackListeners, (the original purpose of this bug) which
> > should be doable.
> Thank you.

:)

Regards,
Piotr
Comment 15 Tobias Liefke CLA 2011-10-12 11:31:14 EDT
I totally agree with Igor and Piotr: 

One of the biggest advantages of RAP is the possibility to develop the Rich Client and the Web Client application simultaneously. 
There are many frameworks that are better for creating pure web clients - so why should I choose RAP? Because I can use the same code and ... now comes the killer argument ... I can choose from a wide range of existing plugins! Wouldn't it be nice if most of these plugins would work in RAP, even if they weren't written for RAP, even if I don't have the sources to adapt them for RAP?

I know, not all of them will work (at least not immediately) because of the gap between Rich and Web Client. But we should try to minimize that gap as far as we can - and it's not as big as you may think.

To give you an example, here what I had to do patch in RAP to bring the BPEL Designer to life:

* NOOps for Control.addMouse*Listener (identically for remove*)
  - the reason for this CR
* NOOps for Control.addPaintListener(PaintListener) 
  - I hope we can add them in the scope of this CR, too?
* Added NOOPs for org.eclipse.swt.widgets.ScrollBar.setIncrement(int) and ScrollBar.setPageIncrement(int)
  - I don't think that any application will stumble if the given increment doesn't affect the browser scrolling
* Added org.eclipse.swt.graphics.GC.textExtent(String string, int flags)
  - Ignores the flags and returns the same as textExtent(String string)
* Added some of the org.eclipse.swt.SWT constants 
  - From my point of view we should add nearly all constants from SWT, because most of them don't create any harm (it's up to the control to decide to throw an IllegalArgumentException or silently ignore if a given argument has the wrong value). Best example is "SHADOW_ETCHED_IN" which is already marked as "this is a HINT" in SWT - but nevertheless missing in RWT.
* Added an own implementation for org.eclipse.swt.dnd.Clipboard
  - For now a dummy, but should use the web session for clipboard emulation
* Added org.eclipse.ui.IEditorRegistry.isSystemExternalEditorAvailable and IEditorRegistry.getSystemExternalEditorImageDescriptor
  - It should be clear that these can just return "false" respectively "null", as this is exactly what is available in the context of RAP
  
Please tell me, for which of these point I should add a new CR for further discussion?
Comment 16 Tobias Liefke CLA 2011-10-12 11:34:03 EDT
BTW: I just opened an issue for another "API breaker": bug 360680 - Don't break IDialogConstants API in RAP
Comment 17 Ralf Sternberg CLA 2011-10-12 16:43:58 EDT
(In reply to comment #13)
> What if one application should run in both worlds: As a "normal" RCP application
> and as a SaaS service deployed on RAP? In that case (we want to do this with
> BPEL designer (http://www.eclipse.org/bpel/)) we (currently) have to patch the
> RAP itself and merge that patch to every new release of RAP without getting any
> extra functionality. So it is quite an effort we have to invest _just_ to be
> able to deploy our application on RAP.

Let me point out here (although I think you are aware of it) that there are alternatives to patching RAP. We usually suggest an approach that involves different fragments for RAP and RCP. The application code calls generic facades, and the fragments (you can also use bundles and OSGi services for example) contribute the platform-specific implementation. Patching RAP should be the last resort, used only when you cannot modify the application code in question.

I'm aware that the fragment approach complicates application development, but single-sourcing almost always requires handing small differences on different platforms, not just because of missing API in RAP, but also because of different requirements of desktop and webclient. So even though the differences are usually very small, I doubt that there are many cases where you can re-use an entire application without any changes.

> IMO each application has to go to through its quality assurance cycle - on each
> platform it is supposed to be run. So those problems should occur early.

I agree that every application _should_ undergo thorough acceptance testing, but that's not always the case.
Moreover, even if you notice a problem, it may be hard to trace the cause of this problem back to the empty API.

> I think that the major issue here is how to inform the developer (of the
> deployed application) that some methods are not really implemented. I personally
> would start with the clear logging pattern. The warning log message like e.g.
> "Warning: This method has been implemented as NOOP." would completely suffice.

We've also considered to provide some tooling that adds markers for those unsupported API. I think if we could find a proper way to do that, we could consider implementing all missing API as no-ops. If you have some ideas here, we're open for suggestions. But neither polluting the logs with warnings nor misusing the @deprecated annotation seem to be very suitable to me. I don't know many people who study logs...
Comment 18 PSko CLA 2011-10-12 17:21:03 EDT
> Let me point out here (although I think you are aware of it) that there are
> alternatives to patching RAP. We usually suggest an approach that involves
> different fragments for RAP and RCP. The application code calls generic
> facades, and the fragments (you can also use bundles and OSGi services for
> example) contribute the platform-specific implementation. Patching RAP should
> be the last resort, used only when you cannot modify the application code in
> question.

This 'Only when' is a very common situation. Lots of plugins you have to review and patch/modify. This process requires a) source code access which is not so common b) refactor to use OSGi or facades.

> 
> I'm aware that the fragment approach complicates application development, but
> single-sourcing almost always requires handing small differences on different
> platforms, not just because of missing API in RAP, but also because of
> different requirements of desktop and webclient. So even though the differences
> are usually very small, I doubt that there are many cases where you can re-use
> an entire application without any changes.

I'm sure there are other people that can state quite opposite. The thing is that you *may* have to have different code for web and standalone, but this is a case where you are aware of differences - like file upload/download vs load/save files - this *is* obvious. We're talking here about features, that are purely cosmetic like moveMoment listeners - for most applications, having noop'ed such functionality will not make any harm - let's say user uses a pen instead of a mouse, and the only thing we can trace is a click on a button (no movements).

> 
> > IMO each application has to go to through its quality assurance cycle - on each
> > platform it is supposed to be run. So those problems should occur early.
> 
> I agree that every application _should_ undergo thorough acceptance testing,
> but that's not always the case.
> Moreover, even if you notice a problem, it may be hard to trace the cause of
> this problem back to the empty API.

Nope, just look at the app logs, and you'll find sth like this:
WARN RWT NOOPed operation.

 
> > I think that the major issue here is how to inform the developer (of the
> > deployed application) that some methods are not really implemented. I personally
> > would start with the clear logging pattern. The warning log message like e.g.
> > "Warning: This method has been implemented as NOOP." would completely suffice.
> 
> We've also considered to provide some tooling that adds markers for those
> unsupported API. I think if we could find a proper way to do that, we could
> consider implementing all missing API as no-ops. If you have some ideas here,
> we're open for suggestions. But neither polluting the logs with warnings nor
> misusing the @deprecated annotation seem to be very suitable to me. I don't
> know many people who study logs...

Hm, I know many many people that study logs to get the idea where the problem may coming from - so I'm amused, who are those people you refer to? 
Once again - if somebody is using SWT, then it uses lots of third-party widgets - you don't have sources, you just try to use it - you don't see any warn in IDE that the method does not exist. All you can do is to use debugger and/or trace logs. The thing with 'misusing' - okay, we can be purists - don't use it because it has been designed for other purposes - but, this is the most basic, the quickest, and easily understandable by developers thing you RWT devs can do. However, I'm okey to have NOOPs and stay with warn logs.

As so far, nobody purposed any other option - time is running, this ticket is 1 year old - I can see next years of bouncing the topic how we can mark NOOP ops... it is a simple waste of time and loosing opportunity to let RWT spread its wings when there isn't so much competition yet.


Regards,
Piotr
Comment 19 Ralf Sternberg CLA 2011-10-12 17:26:08 EDT
(In reply to comment #15)
> I know, not all of them will work (at least not immediately) because of the gap
> between Rich and Web Client. But we should try to minimize that gap as far as we
> can - and it's not as big as you may think.

You're so right. But that's what we do all the time! If you look at our new and noteworty pages you'll find that we implemented a *lot* of missing API in every release to minimize that gap. So, let's look at your list:

> * NOOps for Control.addMouse*Listener (identically for remove*)

MouseTrackListener is covered by this bug. You're free to open an enhancement request for MouseMoveListener, but be prepared to get a WONTFIX if you don't manage to convince us ;-).

> * NOOps for Control.addPaintListener(PaintListener)
> - I hope we can add them in the scope of this CR, too?

No, please open a separate one!
We already have addPaintListener on Canvas (and therefore on all inherited methods). Since it is currently only supported by Canvas, in fact we have this method as empty API on all subclasses of Canvas. And as we plan to extend painting to more widgets, this should be a good argument to implement it on Control.

> * Added NOOPs for org.eclipse.swt.widgets.ScrollBar.setIncrement(int) and
> ScrollBar.setPageIncrement(int)

I guess this might be doable. Please open an enh req.

> * Added org.eclipse.swt.graphics.GC.textExtent(String string, int flags)

Same here.

> * Added some of the org.eclipse.swt.SWT constants
> - From my point of view we should add nearly all constants from SWT, because
> most of them don't create any harm (it's up to the control to decide to throw an
> IllegalArgumentException or silently ignore if a given argument has the wrong
> value). Best example is "SHADOW_ETCHED_IN" which is already marked as "this is a
> HINT" in SWT - but nevertheless missing in RWT.

There should be no problem with constants marked as a hint. SHADOW_ETCHED_IN/OUT are already added in HEAD.

> * Added an own implementation for org.eclipse.swt.dnd.Clipboard
> - For now a dummy, but should use the web session for clipboard emulation

C&P in the browser seems to be far out, but at first glance it should be possible to implement Clipboard and Transfer.

> * Added org.eclipse.ui.IEditorRegistry.isSystemExternalEditorAvailable and
> IEditorRegistry.getSystemExternalEditorImageDescriptor
> - It should be clear that these can just return "false" respectively "null",
> as this is exactly what is available in the context of RAP

This looks so reasonable that we opened bug 360736 right away ;-)

Thanks for sharing this list.
Comment 20 Ralf Sternberg CLA 2011-10-12 17:47:50 EDT
(In reply to comment #18)
Piotr,

thanks for your input. It's helpful for our discussion to get other's perspectives at the problem. We will discuss the general issue once again. Just a few comments:

> > Moreover, even if you notice a problem, it may be hard to trace the cause of
> > this problem back to the empty API.
> 
> Nope, just look at the app logs, and you'll find sth like this:
> WARN RWT NOOPed operation.

... and you get used to this kind of warnings and don't read them anymore. How would you notice that it's now one more than before?

> Hm, I know many many people that study logs to get the idea where the problem
> may coming from - so I'm amused, who are those people you refer to?

As far as I see, logging is useful for deployed applications, but during development, debugging is a more appropriate means of getting this idea. But let's not get into an argument here. As I said, I understand your points, but logging will not be an option for us. Sorry.

> As so far, nobody purposed any other option - time is running, this ticket is 1
> year old - I can see next years of bouncing the topic how we can mark NOOP
> ops... it is a simple waste of time and loosing opportunity to let RWT spread
> its wings when there isn't so much competition yet.

I'm sorry but it's open source software. Our time is limited and there are lots of bugs to choose from. For every user, another bug is the most important...
Comment 21 PSko CLA 2011-10-12 18:06:13 EDT
Ralf,

Thanks for your comment :)

(In reply to comment #20)

> I'm sorry but it's open source software. Our time is limited and there are lots
> of bugs to choose from. For every user, another bug is the most important...


Yup, I understand that - however, this bug(s)/issue is so important to me so much, as a year go started to patch RWT for my own as other people do. But, as we can see, more and more people start to bounce the topic, so... the question is: whether is it really wise to ignore NOOP approach (which does not require substantial effort) and risk misfire of RWT as an option to implement web apps? 
BTW, do you think Wine project would grow up to this point if they were to ignore undocumented or 'hard' to implement functionality of Win API? They try to implement it without causing the app to die (you can spot lots of warnings when windows app is running via wine that sth is stubbed - most people don't pay attention that there is no 'tooltip' over a button as long as the application does what it should).

Regards,
Piotr
Comment 22 PSko CLA 2011-10-12 18:07:32 EDT
(In reply to comment #21) 
> Yup, I understand that - however, this bug(s)/issue is so important to me so

correction: this bug(s)/issue is *not* so important
Comment 23 Arian Storch CLA 2011-10-13 03:32:49 EDT
I agree with Ralf and Ivan.

Writing empty methods or similar things only to make switches between RCP and RAP easier isn't a good style. The point is though you won't get any errors, the code won't work correct or as expected. Furthermore it's much harder to debug because you have no idea where to start if something goes wrong.

You won't get any compile errors but the code won't work correct.

I wouldn't appreciate such a design.
Comment 24 Tobias Liefke CLA 2011-10-13 10:09:49 EDT
(In reply to comment #19)
> > I know, not all of them will work (at least not immediately) because of the gap
> > between Rich and Web Client. But we should try to minimize that gap as far as we
> > can - and it's not as big as you may think.
> 
> You're so right. But that's what we do all the time! If you look at our new and
> noteworty pages you'll find that we implemented a *lot* of missing API in every
> release to minimize that gap. 

Kudos for that effort. And please don't stop on the last meters, just because it's seems to be "sisyphean" ;-)


> > * NOOps for Control.addMouse*Listener (identically for remove*)
> 
> MouseTrackListener is covered by this bug. You're free to open an enhancement
> request for MouseMoveListener, but be prepared to get a WONTFIX if you don't
> manage to convince us ;-).
>

Nope, the description states MouseMoveListeners as well - pooh, one less to go ;-)

> > * NOOps for Control.addPaintListener(PaintListener)
> > - I hope we can add them in the scope of this CR, too?
> 
> No, please open a separate one!
> We already have addPaintListener on Canvas (and therefore on all inherited
> methods). Since it is currently only supported by Canvas, in fact we have this
> method as empty API on all subclasses of Canvas. And as we plan to extend
> painting to more widgets, this should be a good argument to implement it on
> Control.

bug 360820

> 
> > * Added NOOPs for org.eclipse.swt.widgets.ScrollBar.setIncrement(int) and
> > ScrollBar.setPageIncrement(int)
> 
> I guess this might be doable. Please open an enh req.
> 

bug 360794

> > * Added org.eclipse.swt.graphics.GC.textExtent(String string, int flags)
> 
> Same here.

bug 360814

> 
> > * Added some of the org.eclipse.swt.SWT constants
> > - From my point of view we should add nearly all constants from SWT, because
> > most of them don't create any harm (it's up to the control to decide to throw an
> > IllegalArgumentException or silently ignore if a given argument has the wrong
> > value). Best example is "SHADOW_ETCHED_IN" which is already marked as "this is a
> > HINT" in SWT - but nevertheless missing in RWT.
> 
> There should be no problem with constants marked as a hint.
> SHADOW_ETCHED_IN/OUT are already added in HEAD.
> 

I added bug 360830 for the line styles, others might follow.

> > * Added an own implementation for org.eclipse.swt.dnd.Clipboard
> > - For now a dummy, but should use the web session for clipboard emulation
> 
> C&P in the browser seems to be far out, but at first glance it should be
> possible to implement Clipboard and Transfer.
> 

bug 360790

> > * Added org.eclipse.ui.IEditorRegistry.isSystemExternalEditorAvailable and
> > IEditorRegistry.getSystemExternalEditorImageDescriptor
> > - It should be clear that these can just return "false" respectively "null",
> > as this is exactly what is available in the context of RAP
> 
> This looks so reasonable that we opened bug 360736 right away ;-)
> 
> Thanks for sharing this list.

Thank you for the prompt answers.
Comment 25 Tobias Liefke CLA 2011-10-13 10:44:59 EDT
(In reply to comment #23)
> 
> You won't get any compile errors but the code won't work correct.
> 

So what about introducing our own annotation "@Ineffective" for implemented NOOP methods and for constants without meanings? That is exactly what annotations are for and would have many advantages:
1. We don't have to use "@Deprecated" (which is misleading).
2. We won't get NoSuchMethodErrors or similar for (closed) third party plugins that were compiled against SWT and that are using the unimplemented methods.
3. We can supply a "annotation processor" which is configurable to either throw an compiler error (your requested behavior) or print a warning during compilation. That processor can be integrated in Eclipse (with the RAP tooling project), but available for an external compiler, too, for use with Ant, Maven or similar.
4. We could explicitly suppress any compile problem for a specific usage already tested to be sufficient for the application - e.g. with @SuppressWarnings("rap.missing").
5. Using a second annotation "@Incomplete" we could even introduce a new level and describe existing implementations that are not fully equivalent to the SWT functionality.
6. The RWT team has a quick overview of open/incomplete implementations for every class (even a report could be created).

If you and the others think that this is a approach we could investigate further on, I would suggest to create a new issue and add the proposed annotation and a first implementation of a processor to it, so we can continue any discussions there and close this issue right here as soon as the Mouse*Listeners are added to Control.
Comment 26 Igor Novakovic CLA 2011-10-13 11:09:45 EDT
(In reply to comment #25)
> > You won't get any compile errors but the code won't work correct.
> >
> 
> So what about introducing our own annotation "@Ineffective" for implemented NOOP
> methods and for constants without meanings?
+1

Cheers
Igor
Comment 27 Austin Riddle CLA 2011-10-13 11:40:44 EDT
(In reply to comment #25)

> So what about introducing our own annotation "@Ineffective" for implemented
> NOOP methods and for constants without meanings? 

+1 for a new annotation in RWT. 
I would like to make a few points:
1. Although RAP attempts to implement all of the APIs it can, there is no _explicit_ version matching to corresponding org.eclipse.* api versions. That means that to achieve binary compatibility with other eclipse plugins, you would have to have facade bundles to define the versions you are matching.
2. In our shop, we patch RAP to add certain missing API and do find it cumbersome to maintain. I also maintain the RAP GEF port which currently uses a supplemental bundle to add API that is missing in RAP. However, this becomes a problem if there are other project ports that utiliize the same api. 

I think that a new annotation is the way to go because it is tooling friendly. I agree with Ralf that logging is not the best option as a platform-wide solution.
Comment 28 Austin Riddle CLA 2011-10-13 11:46:12 EDT
Hello,

I have one more note.  When RAP moves to Git, the world changes a bit. It may unsettle some, but after the migration there will be many _flavors_ of RAP in the world.
Comment 29 Arian Storch CLA 2011-10-14 03:50:24 EDT
I also believe that a new annotation could be a practible way.
Comment 30 Ralf Sternberg CLA 2011-10-15 15:37:49 EDT
I think the major effort will be the tooling that finds those annotations and creates markers. We also thought about throwing UnsupportedOperationExceptions that can be turned on or off by a system property.

Tobias, if you can come up with a simple implementation, your help is appreciated. But it can take some time until we find the time to integrate it. However, feel free to open an enhancement request.
Comment 31 Austin Riddle CLA 2011-10-17 10:34:11 EDT
(In reply to comment #30)
> Tobias, if you can come up with a simple implementation, your help is
> appreciated. But it can take some time until we find the time to integrate it.
> However, feel free to open an enhancement request.

I am willing to help with this effort, as it would ease some of our burdens.
Comment 32 Andreas Blochberger CLA 2011-10-17 11:15:08 EDT
In my case, this is an error, since we have set up our compiler to treat missing @Override as errors. Now we have a problem with our SWT custom controls, that are derived from Composite or Canvas (here the getBackgroundColor-method is missing in RAP)

In the RCP plugin, the code needs an @Override annotation, but in RAP, this does not compile, since there is no method, that will be overridden. What can we do? The only solution would be to treat missing @Overrides as warnings and ignore them in the RAP version. But that leads to bad code.

What should we do here?
Comment 33 Ralf Sternberg CLA 2011-10-17 18:52:16 EDT
(In reply to comment #32)
Andreas, could you please open a separate bug for this issue? Thanks.
Comment 34 Andreas Blochberger CLA 2011-10-18 03:56:07 EDT
(In reply to comment #33)
> (In reply to comment #32)
> Andreas, could you please open a separate bug for this issue? Thanks.

For what issue? It's about missing interface compatibility breaks singe source efforts. I would rename this bug.

I like your proposed solution with the optional UnsupportedOperationException from comment #30. But in some cases, this is hard to implement (getBackgroundColor must return some value that makes sense)
Comment 35 Ralf Sternberg CLA 2011-10-18 07:31:38 EDT
(In reply to comment #34)
> For what issue? It's about missing interface compatibility breaks singe source
> efforts. I would rename this bug.

Since I didn't quite understand the problem, I asked you to explain it on a separate bug instead of blowing up this one even more. This bug is still about MouseTrackListerners.

What I didn't get about your issue: There is no method getBackgroundColor() on Control, Composite, or Canvas. You're probably referring to getBackground(). But where is this method missing? What difference between SWT and RAP causes your @Override annotations to fail?
Comment 36 Tobias Liefke CLA 2011-10-20 10:32:47 EDT
(In reply to comment #25)

So I created now bug 361551, which contains my proposal of the new annotation and includes an example of a working "annotation checker" as well. 

I didn't add a solution for comparing SWT and RWT API for consistency (I could, if you really need it). Neither did I add a tool for generating the NOOPs automatically, as its not a trivial task to decide how the NOOP looks in every case - sometimes you need at least to save the given value in setters to return them in getters, even if they have no effect.

From my point of view the RWT team should now just add the NOOPs for add/removeMouse*Listener and close this issue here. As soon as the annotations are available, they should be used for marking these new methods.

(In reply to comment #32)
Andreas: I think that the proposed annotations would solve your problem too, as the overridden methods are available then in RAP (but marked as "@Ineffective").
Comment 37 Igor Novakovic CLA 2011-11-02 06:03:06 EDT
Did someone of RAP-Devs take a look at Tobias' contribution (provided in  bug 361551)?

Cheers
Igor
Comment 38 Ivan Furnadjiev CLA 2011-11-05 01:49:14 EDT
(In reply to comment #37)
> Did someone of RAP-Devs take a look at Tobias' contribution (provided in  bug
> 361551)?
Hi Igor, not yet... This week we were busy with EclipseCon Europe. We won't make it next week too as it is M3 release date.
Comment 39 Igor Novakovic CLA 2011-11-07 03:55:05 EST
(In reply to comment #38)
> Hi Igor, not yet... This week we were busy with EclipseCon Europe. We won't make
> it next week too as it is M3 release date.
Thank you for the info!

BTW: I hope you had a successful conference.
Comment 40 Nick Mussin CLA 2012-03-08 06:14:37 EST
*** Bug 373631 has been marked as a duplicate of this bug. ***
Comment 41 Ivan Furnadjiev CLA 2014-11-25 06:49:20 EST
*** Bug 355373 has been marked as a duplicate of this bug. ***