Hi Vadim,

Vadim Zhukov wrote on Tue, Apr 17, 2018 at 10:06:07AM +0300:

> First of all thank you for taking care of this.

You are welcome.  We would hardly be able to do this without
the work you did on it in the past.

> It's a pity I can't take enough part in workflow for now.

It happens that people have more or less leisure at different points
in time; i hope you are well.

> I'm sorry that you have to work on this.

Don't worry.  We would be glad to see more of you round here,
though, of course.  :)

> Now to the patch. TL;DR: I'm agree with all changes, but a few libs
> should have major bump since ABI is changed (public constants): kdeui,
> khtml, kjs.

Thanks for the review (also to jasper@) and for catching that!

>> Index: patches/patch-khtml_ecma_kjs_events_cpp
>> ===================================================================
>> RCS file: patches/patch-khtml_ecma_kjs_events_cpp
>> diff -N patches/patch-khtml_ecma_kjs_events_cpp
>> --- /dev/null   1 Jan 1970 00:00:00 -0000
>> +++ patches/patch-khtml_ecma_kjs_events_cpp     17 Apr 2018 04:43:42 -0000
>> @@ -0,0 +1,14 @@
>> +$OpenBSD$
>> +
>> +Index: khtml/ecma/kjs_events.cpp
>> +--- khtml/ecma/kjs_events.cpp.orig
>> ++++ khtml/ecma/kjs_events.cpp
>> +@@ -405,7 +405,7 @@ DOM::Event toEvent(const Value& val)
>> +   FOCUS         4096                DontDelete|ReadOnly
>> +   BLUR          8192                DontDelete|ReadOnly
>> +   SELECT        16384               DontDelete|ReadOnly
>> +-  CHANGE        32768               DontDelete|ReadOnly
>> ++  CHANGE        -1                  DontDelete|ReadOnly
>> + @end
>> + */
>> + DEFINE_CONSTANT_TABLE(EventConstants)

> And same for libkjs.

Not sure that is part of the API or ABI - at least "CHANGE" does
not appear in /usr/local/include/kde/.  Besides, -1 and 32768 are
the same "short" number in binary.  But there is no need to split
hairs over it, bumps are not that expensive, and better safe than
sorry, so i bumped kjs, too.

Yours,
  Ingo

Reply via email to