Re: [PATCH 4/6] mshtml: Delegate event construction to functions by type.

[Gabriel Ivăncescu]

On 23/05/2022 19:06, Jacek Caban wrote:
> Hi Gabriel,
>
> On 5/23/22 17:22, Gabriel Ivăncescu wrote:
> > Instead of hardcoding it, which will be necessary as the amount of event
> > types implemented grows.
> > Signed-off-by: Gabriel Ivăncescu
> >
> > dlls/mshtml/htmlevent.c | 33 ++++++++++++++++++++++++---------
> >   1 file changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c
> > index f28dd63..c25b85f 100644
> > --- a/dlls/mshtml/htmlevent.c
> > +++ b/dlls/mshtml/htmlevent.c
> > @@ -2373,6 +2373,26 @@ static BOOL check_event_iface(nsIDOMEvent 
> > *event, REFIID riid)
> >       return TRUE;
> >   }
> > +static DOMEvent *message_event_ctor(nsIDOMEvent *nsevent, 
> > dispex_static_data_t **dispex_data)
> > +{
> > +    DOMMessageEvent *message_event = 
> > heap_alloc_zero(sizeof(*message_event));
> > +    if(!message_event)
> > +        return NULL;
> > -
> >
> > +    message_event->IDOMMessageEvent_iface.lpVtbl = &DOMMessageEventVtbl;
> > +    message_event->event.query_interface = 
> > DOMMessageEvent_query_interface;
> > +    message_event->event.destroy = DOMMessageEvent_destroy;
> > +    *dispex_data = &DOMMessageEvent_dispex;
> > +    return &message_event->event;
> > +}
> > -
> >
> > +static const struct {
> > +    DOMEvent* (*proc)(nsIDOMEvent*,dispex_static_data_t**);
> > +    compat_mode_t min_mode;
> > +} event_type_ctors[ARRAY_SIZE(event_types)] = {
> > +    [EVENT_TYPE_MESSAGE]        = { message_event_ctor, 
> > COMPAT_MODE_QUIRKS },
> > +};
>
> While we may need something more generic at some point, this seems this 
> patch series just uses it just for two event types and it's not clear we 
> want to go in that direction. The way we handle DOMCustomEvent is 
> probably better in cases where it's possible. At this point you leave 
> out of generic mechanism, meaning that we will prefer to not use the new 
> mechanism anyway. I'd suggest to wait for more problematic cases before 
> having something like that.

Do you mean checking for Gecko's interface? AFAIK there is no 
nsIDOMProgressEvent (it seems to have been removed?) so I can't do that.

So what should I do? Add it inline like for Message Event? I heavily 
dislike hardcoding checks but...

Thanks,
Gabriel