[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