[Development] Header diff for QtGui
Knoll Lars
Lars.Knoll at digia.com
Thu Jun 27 14:22:51 CEST 2013
Ok, explanation sounds reasonable to me, and given that the classes were
documented as internal this is acceptable IMO.
Cheers,
Lars
On 6/27/13 11:03 AM, "Frederik Gladhorn" <frederik.gladhorn at digia.com>
wrote:
>Since we were unsure about the accessibility classes we had QAccessible*
>marked as internal documentation wise, but sadly not moved the headers to
>be
>_p.h.
>The plan is to make them public for 5.2.
>
>Onsdag 26. juni 2013 14.44.48 skrev Thiago Macieira:
>> One issue found:
>>
>> On quarta-feira, 26 de junho de 2013 13.49.40, Thiago Macieira wrote:
>> > class Q_GUI_EXPORT QAccessibleInterface
>> > {
>> >
>> > +protected:
>> > + virtual ~QAccessibleInterface();
>> > +
>> >
>> > public:
>> > - virtual ~QAccessibleInterface() {}
>>
>> SC/BC BREAK
>>
>> Change in protection level is not acceptable (MSVC and Sun CC mangle the
>> protection level). The constructors are public, so it's possible to
>>derive
>> from this class.
>>
>> The destructor should go back to public.
>>
>> This is also an SC break, because it makes it impossible to delete a
>> QAccessibleInterface.
>
>This was done intentionally since the class is currently internal and we
>want
>to prevent mis-use in the future (the interfaces are managed in a cache
>and
>should never be deleted by the user).
>This breakage was done intentionally since so far no one should have used
>the
>class.
>In my opinion making the dtor public is a step back and I seriously doubt
>that
>anyone is using this at the moment.
>
>>
>> The rest are comments:
>> > class Q_GUI_EXPORT QAccessible
>> >
>> > -#ifndef qdoc
>> > - :public QObject
>> > -#endif
>> >
>> > {
>> >
>> > - Q_OBJECT
>> > - Q_ENUMS(Role Event State)
>> > + Q_GADGET
>> > + Q_ENUMS(Role Event)
>>
>> Acceptable because QAccessible's only constructor is private. The
>>change in
>> class size and vtable will not affect anyone.
>>
>> The change from Q_OBJECT to Q_GADGET means the virtual functions are
>>gone
>> now (qt_metacall, qt_metacast, metaObject), but no one could call them
>> before either.
>>
>> The tr() functions are gone too. That's the only question I have,
>>because it
>> was possible to call:
>> QAccessible::tr("Hello");
>That would be misusing the tr function since the context should be the
>actual
>context not a random one (almost as bad as QObject::tr which is a no-no
>unless
>the string is in QObject).
>
>>
>> Opinions?
>>
>> > --- a/src/gui/kernel/qwindow.h
>> > +++ b/src/gui/kernel/qwindow.h
>> >
>> > @@ -261,10 +286,12 @@ public Q_SLOTS:
>> > void setWidth(int arg);
>> > void setHeight(int arg);
>> >
>> > - void setMinimumWidth(int w);
>> > - void setMinimumHeight(int h);
>> > - void setMaximumWidth(int w);
>> > - void setMaximumHeight(int h);
>> > + Q_REVISION(1) void setMinimumWidth(int w);
>> > + Q_REVISION(1) void setMinimumHeight(int h);
>> > + Q_REVISION(1) void setMaximumWidth(int w);
>> > + Q_REVISION(1) void setMaximumHeight(int h);
>>
>> The meta object system doesn't care about Q_REVISION. It will simply
>>extract
>> the information. So this is neither BIC nor SIC.
>>
>> However, the information is relevant to QML. QML experts, please
>>comment on
>> whether the change above is safe.
>If it was in the 5.0 release, I would say the properties should still be
>there
>and thus the Q_REVISION be removed since the classes otherwise suddenly
>lose
>the setters.
>
>--
>Best regards,
>Frederik Gladhorn
>Senior Software Engineer - Digia, Qt
>Visit us on: http://qt.digia.com
>
>_______________________________________________
>Development mailing list
>Development at qt-project.org
>http://lists.qt-project.org/mailman/listinfo/development
More information about the Development
mailing list