100115 - nsIDocument.h (content) brings in nsEvent.h (widget)

Status: RESOLVED WONTFIX

(Core :: Layout, defect, P2)

Description

[Alec Flett]

Another ugly dependency

Comment 1

[Alec Flett]

we include nsEvent.h for these two definitions:

enum nsEventStatus {  
    /// The event is ignored, do default processing
  nsEventStatus_eIgnore,            
    /// The event is consumed, don't do default processing
  nsEventStatus_eConsumeNoDefault, 
    /// The event is consumed, but do default processing
  nsEventStatus_eConsumeDoDefault  
};

struct nsEvent;

----

we can forward-declare nsEvent, but I'm not sure about nsEventStatus.

Comment 2

[Jon Smirl]

The other big culprit for including nsEvent.h is nsView.h.
The IDocument problem is causing XSLT to depend on widget.

What about moving nsEvent.h to dist/include and making it ownerless?

Comment 3

[Jon Smirl]

Also C++ does not allow enums to be forward declared. The issue is currently 
being debated by the C++ standards people.

Comment 4

[Alec Flett]

no, dist/include is going away. all include files must have a module.
in the case of nsIDocument, it looks like the only nsEvent stuff is for printing
- I'm thinking we might want to break nsIDocument into two interfaces for that

I'm not sure about nsIView. Quite frankly, I'm surprised that HandleEvent isn't
available in another interface and that it has to be in in nsIView

Comment 5

[Alec Flett]

moving tracking/non-critical bugs to mozilla 1.2

Comment 6

[John Keiser (jkeiser)]

nsEventStatus should be in content or something content depends on.  Everyone
who uses this is deeper down the chain than content: layout, view, widget, tools ...

I think it should be moved into content/events/public.

Of course, widget/ doesn't currently have a REQUIRES at all, unless I simply
missed it.  Perhaps these event definitions require a library or directory of
their own after all.  events/ perhaps?

Comment 7

[Brian Ryner (not reading)]

I don't personally see a problem with this dependency.

Comment 8

[John Keiser (jkeiser)]

The problem, in short, is modularity.  It's an unnecessary dependency, and if
someone theoretically wanted to replace our layout and rendering stuff but keep
content (or remove the layout altogether), they couldn't do it (not that they
could now, but theoretically they ought to be able to).

Widget converts platform events into events our DOM can handle.  Content doesn't
need to care how the events get to it, but it *does* need to care what format
the events are in.  Therefore the event struct definition should be in content
and widget should use it.

Comment 9

[John Keiser (jkeiser)]

Assuming nsEvent.h *does* make sense to move, nsGUIEvent.h does too.

Comment 10

[John Keiser (jkeiser)]

Moving this into content/events makes it so we can remove the widget/ dependency
entirely from 60-70 directories, and *adds* the dependency on content/ to 15
directories or so (many of them in widget/).

bryner saith widget should not depend on content, which is ok--perhaps it is
just a "generic windowing system hookup library."  (In which case there is
rethinking: it already depends on content in several places.)  But content
should still not depend on widget.  The alternative is to make a "generic event
hookup library" such that an event consumer could extend interfaces like
nsIMouseListener and receive events, and the only real hookup between content
and widget would be the place (esm?) where the consumer is added as a listener
to the library.

Perhaps a toplevel directory like events/ would do.  Files that would go there
that I can see:

nsEvent.h
nsGUIEvent.h
nsIMouseListener.h
nsIEventListener.h

This would simply replace the dependency on widget/ with that on events/ in most
cases, and *add* the dependency on events/ in most widget directories.

I have noticed, however, that there are 5-6 directories where we are
#include'ing things like nsEvent.h and nsIWidget.h when we do not need to.  In
those places the widget dependency goes away entirely.

I would be happy to do this work if we (bryner and alecf?) agree it's ok.  There
are a few other items of dependency cleanup that appear in the process (for
example using forward declarations of classes in include files instead of
#including their definitions).

Comment 11

[Benjamin Smedberg]

This is false modularity. These are both tier-9/gecko modules and there's no
reason why they shouldn't be able to depend on eachother.