Opened 9 years ago

Closed 5 years ago

Last modified 4 years ago

#2897 closed enhancement (fixed)

Add wxICON_NONE style for wxMessageBox

Reported by: nickjacobs Owned by: Peter_Most
Priority: low Milestone:
Component: GUI-all Version:
Keywords: wxMessageDialog wxMessageBox wxICON_NONE simple Cc: nickjacobs, abxabx, Peter_Most@…, pavel_mendl@…
Blocked By: Blocking:
Patch: yes

Description

If the caller to wxMessageBox explicitly specifies a
"style" parameter with none of wxICON_EXCLAMATION,
wxICON_HAND, wxICON_INFORMATION, wxICON_QUESTION, then
none of them should be in the message box. However,
wxMessageBox forces either wxICON_QUESTION or
wxICON_INFORMATION.
This is wrong behaviour because it conflicts with what
the user explicitly specified. It is also inconsistent
with the documentation.

Attachments (2)

ticket_2987.patch download (581 bytes) - added by Peter Most 6 years ago.
Check for wxYES_NO in wxMessageBox
ticket_2897.patch download (4.7 KB) - added by Peter Most 5 years ago.
Add/Support wxICON_NONE

Download all attachments as: .zip

Change History (26)

comment:1 Changed 9 years ago by vadz

I think having a default icon is very convenient, why would
you want to not have an icon in the msg box? If this is
really a serious concern, then a wxICON_NONE flag could be
added but I'd like to know why it is a problem.

comment:2 Changed 9 years ago by nickjacobs

re: a default icon is convenient
Then make the default for the style parameter
wxOK|wxICON_INFORMATION.

re: why would you want to not have an icon?
The default icon is big and takes up valuable screen real
estate. But I find the question inappropriate. By setting
all the icon bits off in the style parameter, the caller is
telling wxMessageBox not to display an icon. "Why" is the
caller's business, not wxMessageBox's.

re: wxICON_NONE
This would be completely inappropriate. There is already a
means for the caller to specify no icon - an explicit style
parameter with none of the wx_ICONxxx bits set. Just do what
the user has specified.

As far as I can see, the fix is trivial for all platforms
except gtk.

If (for whatever reason) you choose to ignore what the
caller specifies, then at a minimum you should change the
documentation, to tell the user that specifying no icon will
result in a wxICON_INFORMATION.

comment:3 Changed 9 years ago by vadz

Then make the default for the style parameter

wxOK|wxICON_INFORMATION.

This wouldn't allow to have the right icon by default for
wxYES_NO.

"Why" is the caller's business, not wxMessageBox's.

The library API has to be not only consistent but also easy
to use. Personally I definitely find it easier if I don't
have to specify (the usually redundant) icon flags in the code.

As far as I can see, the fix is trivial for all platforms

except gtk.

Which brings us to the point of consistency. As there must
be an icon under GTK+ why not in the other ports?

The default icon is big and takes up valuable screen real

estate.

The default icon is also what the user is used to see, any
application not using it looks strange and foreign.

But I find the question inappropriate.

I find this strange. Any API decision should be based on
some arguments, how else can you be sure to make the right
decisions?

If you want to continue this discussion, please let's do it
on wx-dev, it's more convnient there.

comment:4 Changed 9 years ago by nickjacobs

There doesn't seem much point to continuing the discussion
because your mind seems closed. I assume you'll at least
change the documentation, though.

By the way, your comment "As there must be an icon under
GTK+" is just plain wrong. You can't use the GTK+ messagebox
if you don't want an icon, that's all. That's why I said the
change was trivial on all platforms except GTK+. It's still
perfectly possible on GTK+; it just requires a little more code.

comment:5 Changed 9 years ago by vadz

There doesn't seem much point to continuing the discussion

because your mind seems closed.

You didn't give any good arguments in favour of your point
of view so far and this certainly is not going to count neither.

"As there must be an icon under GTK+" is just plain wrong.

You can't use the GTK+ messagebox if you don't want an icon,
that's all.

Please, let's be serious. Calling something "plain wrong"
when it's correct up to some nitpicking (of course
wxMessageBox should use the native message box, otherwise
what's the point of having it?) is not going to win you any
arguments (nor friends) neither.

comment:6 Changed 9 years ago by abxabx

BTW: adding style is result of patch #940683 titled
"wxMessageBox autocompletion" and this patch was result of
the discussion
http://lists.wxwidgets.org/cgi-bin/ezmlm-cgi?5:sss:46498 . I
agree the current behaviour is better and users of a few
releases didn't compained about it (wxMessageBox is used
quite often) so it seems handy.

ABX

comment:7 Changed 9 years ago by nickjacobs

If you consider the behaviour described to be a feature, not
a bug, in the code, then the documentation of wxMessageBox
should be changed.

comment:8 Changed 9 years ago by abxabx

If you are the one who finds it not clear I would prefer if you
could provide first draft of the wording you would like to have
there (also because I'm not fluent in english). You could add a
note there that lack of icon can be easily achieved using
wxMessageDialog directly as it's done in a few places of
samples. Note you can use:
http://wiki.wxwidgets.org/docpatcher/

Thanks in advance!

ABX

comment:9 Changed 9 years ago by nickjacobs

If you are the one who finds it not clear I would prefer if you
could provide first draft of the wording you would like

I find the documentation perfectly clear. And I agree with
what it says. It is you who assert that the code is correct,
which implies that the documentation is wrong.

Personally, as a developer, I like to specify the functionality
first, then implement what is documented. The alternative
method - write some code first, then document whatever it
does, bugs and all - is not something I want to be a part of,
thanks. Of course you are perfectly entitled to a different
opinion. But I do think it would help all your users if you
brought the documentation and the code into agreement, one
way or another.

comment:10 Changed 9 years ago by abxabx

I find the documentation perfectly clear.

Just like me. The only sentence about style in documentation
says "style may be a bit list of the following identifiers". This
is valid statement because style doesn't recognize other bits.
If you want it to be more precise regarding internals please
provide suggested wording you like best, but if you do not
know how to improve it I'm not sure what more I can do for
you :-(

Personally, as a developer, I like to specify the functionality

You must be new in open source and in wxWidgets if you
have need to speak about your developer experience. Thanks
for your bug report but I do not see where the bug is so if you
find something broken please write in details new wording of
the documentation or mark or repost your problem as feature
request.

I do think it would help all your users if you
brought the documentation and the code into agreement,

one

way or another.

Thanks for this nice advice. We try our best to do it all the
time. We are open for documentation patches where things
needs more improvements and when we do not see any
conflict between code and documentation but real users see
it. Thanks in advance for bringing such patch to us.

ABX

comment:11 Changed 7 years ago by wojdyr

  • Component set to GUI-all
  • Keywords wxMessageDialog wxMessageBox added

comment:12 Changed 7 years ago by Peter Most

  • Cc Peter_Most@… added

I can understand both sides. As a developer I would expect it to behave like nickjacobs has described, but as a user I wouldn't want a dialog without an icon.
Why not describe what is going to happen if no icon has been specified? Something like:

"If none of the above wxICON_xxx symbols is specified, then a default symbol will be shown."

We could even describe how the default symbol is selected:

"If the wxYES_NO style has been specified, then the wxICON_QUESTION symbol will be shown otherwise the wxICON_INFORMATION symbol."

BTW: The code to select the default icon is only testing the wxYES flag. Shouldn't it test both the wxYES and wxNO flags e.g.:

decorated_style |= ( style & wxYES_NO ) ? wxICON_QUESTION : wxICON_INFORMATION ;

Because if I call wxMessageBox (wxGTK2) like this:

wxMessageBox( wxT( "Continue?" ), wxT( "Title" ), wxNO, frame );

I get a message box with the Yes/No buttons and an information symbol!
And then I also have to ask why does it show Yes/No buttons if i only specified the wxNO style? ;-)

-- Peter

comment:13 follow-up: Changed 7 years ago by vadz

  • Cc vadz removed
  • Resolution set to fixed
  • Status changed from new to closed

It doesn't make sense to specify wxNO without wxYES, I guess we could add an assert for it.

Anyhow, I've documented the existing behaviour in r54325 so I'm finally closing this bug (I had no idea it was still opened, I guess being unpleasant really works in the sense that I was sufficiently riled by nickjacobs comments to not even do this 3 years ago...).

Changed 6 years ago by Peter Most

Check for wxYES_NO in wxMessageBox

comment:14 in reply to: ↑ 13 Changed 6 years ago by Peter Most

  • Patch set

Replying to vadz:

It doesn't make sense to specify wxNO without wxYES, I guess we could add an assert for it.

There already is an assert in /include/wx/msgdlg.h(84) and I agree it doesn't make sense. I tried to come up with a dialog/question where I only needed the NO-button and I couldn't come up with one.

But none the less, I would find it nice, if the message box would be smart enough to still show the wxICON_QUESTION for just wxNO even though it doesn't make sense. It also makes the message box more robust and works again in favor of the user which is alway good.

And since I've already have a patch maybe you could still apply it?

-- Peter

comment:15 Changed 6 years ago by vadz

No, sorry, I don't like this patch, it gives the impression, to someone reading the code, that wxNO can be used alone which is wrong because it can not.

comment:16 in reply to: ↑ description ; follow-up: Changed 6 years ago by pmendl

  • Cc pavel_mendl@… added
  • Keywords wxICON_NONE added
  • Patch unset
  • Priority changed from low to normal
  • Resolution fixed deleted
  • Status changed from closed to reopened

I do not want to get into long haggling/disputation above. From my point of view like a user-application developer this bug/feature is definitely not closed (as I arrived here in search before posting bugreport myself).

I am using library wxWidgets 2.8.9 and appropriate help documentation.

I am using wxMessageBox for the About copyright notes. Exactly as mentioned above, I placed no wxICON_* definition in expectation, that I get a frame filled with the given text and got wxICON_INFORMATION to the left of that text, what is completely inappropriate for About text.

Then I found this bugreport thread and tried the recommended workaround with wxICON_NONE. I did not find any mention of this flag not only in help file, but even searching through wx header files. So I must resign for now for displaying wxMessageBox with no icon, what will look quite funny in the About message.

Definitely this bug/feature is not fixed at all so it should not be closed:
1) If no icon flag => no icon appears behaviour will not be adopted for default (what was at least from my point of view really the expected behaviour) there SHOULD be some workaround how to achieve clean text with no implicit icons where needed.

2) (Nonstandard) implicit icon behaviour SHOULD be properly documented in the help file, and not only in bug tracker as the users last resort.

3) The pointings to the workaround SHOULD be properly described (or at least noted by e.g. link) in the library help documentation AND VALID.

This is just my point of view; but it is me, willing to contribute to community at least by the precise bugreporting. How many my predecessors resign quietly I can not say, but take this text as response to the "no user complained so far" argument.

comment:17 in reply to: ↑ 16 Changed 6 years ago by vadz

  • Keywords simple added
  • Priority changed from normal to low
  • Status changed from reopened to confirmed

Replying to pmendl:

I am using wxMessageBox for the About copyright notes. Exactly as mentioned above, I placed no wxICON_* definition in expectation, that I get a frame filled with the given text

This expectation is wrong because of at least 2 reasons: first, many people do expect to have an appropriate icon for wxYES_NO dialog boxes without having to explicitly specify wxICON_QUESTION. Second, some platforms (definitely GTK+, and I believe OS X too) simply do not support message boxes without icons at all so any developer familiar with them wouldn't expect wx to support this neither.

and got wxICON_INFORMATION to the left of that text, what is completely inappropriate for About text.

I know of literally hundreds Windows programs which use the information icon in their about message box. So the above is definitely your subjective assessment and not a statement fact you make it appear to be.

It could also be argued that if you really care about your about dialog appearance you should use wxAboutDialog or a custom dialog for it instead of a message box which, surprisingly, is meant to display a message.

Then I found this bugreport thread and tried the recommended workaround with wxICON_NONE.

This is a misunderstanding, I proposed this as a possible workaround but it was never implemented. I still would be ready to accept a patch adding wxICON_NONE flag (with the appropriate changes to the documentation, including the clear mention that it doesn't work under wxGTK and probably wxMac and so that using this flag will result in different behaviours under different platforms) but this is very low priority and personally I have no plans to work on this.

comment:18 Changed 6 years ago by vadz

  • Summary changed from wxMessageBox ignores style parameter to Add wxICON_NONE style for wxMessageBox
  • Type changed from defect to enhancement

comment:19 Changed 6 years ago by Peter Most

  • Owner set to Peter Most
  • Status changed from confirmed to accepted

comment:20 Changed 5 years ago by Peter Most

  • Patch set

I've tested the changes under wxGTK and wxMSW, but I didn't know which sample to modify to demonstrate the wxICON_NONE flag with wxMessageBox. So to test it I modified my local minimal sample. If there is a better place or a good idea which sample (dialogs?) would be best, then please let me know and I will complete this patch or add another one.

Regards Peter

comment:21 follow-up: Changed 5 years ago by vadz

  • Status changed from accepted to infoneeded

According to Gnome svn GTK_MESSAGE_OTHER was added sometimes during GTK+ 2.9 time line so it is probably only available in 2.10+ and hence its usage should be bracketed by both the corresponding compile-time guards (#ifdef __WXGTK210__) and run-time checks, could you please add them?

Also, why did the values of all the existing wxICON_XXX constants have to change? It seems it would be really simpler (and avoid any potential compatibility problems) if wxICON_NONE was simply assigned the next available value.

Thanks!

Changed 5 years ago by Peter Most

Add/Support wxICON_NONE

comment:22 in reply to: ↑ 21 Changed 5 years ago by Peter Most

  • Status changed from infoneeded to accepted

According to Gnome svn GTK_MESSAGE_OTHER was added sometimes during GTK+ 2.9 time line so it is probably only available in 2.10+ and hence its usage should be bracketed by both the corresponding compile-time guards (#ifdef __WXGTK210__) and run-time checks, could you please add them?

Done, although I don't quite understand the need for the runtime check. Care to explain? ;-) I also hope I did the runtime check correctly since I couldn't find any documentation, which would explain the returned 'const gchar *'.

Also, why did the values of all the existing wxICON_XXX constants have to change? It seems it would be really simpler (and avoid any potential compatibility problems) if wxICON_NONE was simply assigned the next available value.

I thought it would be cleaner and I didn't see any compatibility issues. But I'm not so sure anymore, so I changed it as suggested.

Regards Peter

comment:23 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [61234]) added wxICON_NONE and implement support for it in wxGTK (closes #2897)

comment:24 Changed 4 years ago by wxsite

  • Owner changed from Peter Most to Peter_Most
Note: See TracTickets for help on using tickets.