On 24/05/2022 18:35, Jacek Caban wrote:
Hi Gabriel,
On 5/23/22 19:41, Gabriel Ivăncescu wrote:
On 23/05/2022 19:09, Jacek Caban wrote:
Hi Gabriel,
On 5/23/22 17:22, Gabriel Ivăncescu wrote:
Instead of hardcoding each event, since there will be plenty more.
What's an example of those missing ones?
It's not in this patch series, but IE10+ modes will additionally need:
abort, error, loadstart, loadend, progress
Signed-off-by: Gabriel Ivăncescugabrielopcode@gmail.com
dlls/mshtml/xmlhttprequest.c | 47 +++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index b9f6f1f..baef36e 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -94,12 +94,23 @@ static HRESULT return_nscstr(nsresult nsres, nsACString *nscstr, BSTR *p) return S_OK; } +#define EVENTS_LIST \ + X(readystatechange) \ + X(load)
+#undef X +#define X(event) EVENT_##event, +enum { EVENTS_LIST }; +#undef X +#define X(event) L"" #event, +static const WCHAR *events[] = { EVENTS_LIST }; +#undef X
You could avoid all those preprocessor directives by simply using an array of structs.
The problem is the first one is an enum, while second one is actual data, so I can't use a struct to unify this. I wanted to keep it easily in sync, but I can of course separate them without preprocessor if you prefer.
It doesn't sound too bad and you may always use something like C_ASSERT to make sure we don't forget to update one of its parts.
Generally, this could use more generic event code. Reasons for XMLHttpReqEventListener existance are mostly historical, we could for example have a more generic version of Gecko event listener that could be also used instead of htmlevent_listener. I left it separated while changing the code for IE9+ events mostly because it was simple enough. If this is going to need more duplication in the future, I think we should revisit that. That said, I'm generally fine with the approach for now, but I wouldn't plan on extending that much further in this direction.
Jacek
Hi Jacek,
Well with the event mask there will be almost no duplication at all (except the switch() case in the bind function, but that could probably be looked up in a table too, so all the event data is in the table). The point of the mask is to iterate through it, after all, and avoid any duplication there.
This patch is a good example of that; adding additional events requires no extra code except extending the list (macro now) or table (revised) with the new event. Is that not simple enough?
I don't know what else "extending much further" in this direction means though—do you have some examples of the more generic version of Gecko event listener you mentioned? Or how would it look like? I honestly don't know how else this could be handled, since I learned the code as I worked on it. :-)
IMO I think the mask with table is as simple as it gets when it comes to extending it: just add new entries to the table.
Thanks, Gabriel