Details
- Reviewers
graesslin - Group Reviewers
Plasma - Commits
- R129:a4bdedc38abb: [KDecoration] Restore application menu button
Diff Detail
- Repository
- R129 Window Decoration Library
- Lint
Lint Skipped - Unit
Unit Tests Skipped
src/private/decoratedclientprivate.h | ||
---|---|---|
79 | As that's adding new virtuals we either need to move that into a subclass or increase the so version of the private lib (or just ignore, but then distros complain). |
src/private/decoratedclientprivate.h | ||
---|---|---|
79 | Even if the API is public... plasma libs have no ABI stability promise so I think it's easiest to simply bump the so version. That way distributions can't accidently build kwin against the wrong version of libkdecoration and then have odd vtable crashes. And since this is only BIC but not SIC it's only a minor rebuild inconvenience to anyone who's using libkdecoration's public API. That said, a fully backwards compatible addition using a subclass is of course always nicer :) |
I'm for dedicated sub-class. Reason is that this can result in an annoying transition with e.g. breeze compiled against the old and kwin against the new. Let's subclass and cast correctly
- Introduce subclass (admittedly with a quite a long name) to avoid adding new virtuals in exported class
- Add basic autotest that verifies that clicking the app menu button results in an application menu request
src/decoration.cpp | ||
---|---|---|
201 | I don't get this? We've replaced one ABI crash with throwing a runtime exception on calling a pure virtual, which isn't going to be much better. We either want: if d->metaObject() == ApplicationMenuEnabledDecoratedClientPrivate::staticMetaObject() Or to make the methods in the other class non pure virtuals. |
src/decoration.cpp | ||
---|---|---|
201 | I agree, static_cast doesn't make sense. I would do a dynamic_cast/qobject_cast and check for null. |
src/decoration.cpp | ||
---|---|---|
201 | DecoratedClientPrivate is not a QObject, so only option is dynamic_cast |