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

RESOLVED WONTFIX

Status

()

Core
Layout
P2
normal
RESOLVED WONTFIX
16 years ago
14 years ago

People

(Reporter: Alec Flett, Assigned: Alec Flett)

Tracking

Trunk
Future
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

16 years ago
Another ugly dependency
(Assignee)

Updated

16 years ago
Blocks: 100107
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.5
(Assignee)

Comment 1

16 years ago
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

16 years ago
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

16 years ago
Also C++ does not allow enums to be forward declared. The issue is currently 
being debated by the C++ standards people.
(Assignee)

Comment 4

16 years ago
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
(Assignee)

Updated

16 years ago
Priority: -- → P2
Target Milestone: mozilla0.9.5 → mozilla0.9.6
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.9
(Assignee)

Updated

16 years ago
Depends on: 106686
(Assignee)

Updated

16 years ago
No longer blocks: 100107
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.9 → mozilla1.1
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.1alpha → mozilla1.1beta
(Assignee)

Comment 5

16 years ago
moving tracking/non-critical bugs to mozilla 1.2
Target Milestone: mozilla1.1beta → mozilla1.2alpha
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.2alpha → Future

Comment 6

15 years ago
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?
I don't personally see a problem with this dependency.

Comment 8

15 years ago
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

15 years ago
Assuming nsEvent.h *does* make sense to move, nsGUIEvent.h does too.
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

14 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.