bugGNU Octave - Bugs: bug #49644, questdlg displays buttons in...

 
 

bug #49644: questdlg displays buttons in reverse order

Submitter:  Guillaume <gyom>
Submitted:  Fri 18 Nov 2016 04:31:23 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Guillaume Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 22 Nov 2016 08:37:48 PM UTC, comment #16: 

Just a reminder, the patch hasn't been pushed yet.

Mike Miller <mtmiller>
Group Member
Mon 21 Nov 2016 09:04:42 PM UTC, comment #15: 

Okay, I'll push it to the dev branch and close this report.

Rik <rik5>
Group administrator
Mon 21 Nov 2016 08:01:36 PM UTC, comment #14: 

This patch works for me.

The original fix for this bug was made on default (comment #2). Do you want to backport both changes to stable now? It doesn't seem that important to me to go on stable, and it didn't to you when you first fixed this :)

Mike Miller <mtmiller>
Group Member
Mon 21 Nov 2016 07:43:31 PM UTC, comment #13: 

@Mike: Is the btn.patch okay to push?  Should we backport to stable?

Rik <rik5>
Group administrator
Mon 21 Nov 2016 11:46:30 AM UTC, comment #12: 

I'm using KDE and buttons are displayed in the "right" order with Rik's btn.patch.

Guillaume <gyom>
Sat 19 Nov 2016 01:33:15 AM UTC, comment #11: 

Yes, good point. Either ActionRole or ResetRole should be left-aligned for all platform styles. I'll test your patch a little bit later.

To test other dialogs that use the same mechanism I was doing "edit foo.m" where foo.m was a non-existent file, requires the GUI to be running. Also using the "Save File and Run" button in the GUI editor, when the file is in a directory that is not in the load path, triggers a prompt using the same callback mechanism.

Mike Miller <mtmiller>
Group Member
Sat 19 Nov 2016 01:18:44 AM UTC, comment #10: 

I got things to change by setting the default-layout to 3 (GNOME layout).  So setStylesheet does work as you suggested.

What test were you using for the global test?

Is it possible to restrict the setStyleSheet to this widget itself by using qWidget::setStyleSheet?  There is a possibility that the bare function call is actually doing qApplication::setStyleSheet.

I grepped through the m-files and I didn't see any use of questdlg.  That makes me think we could hack just the questdlg to get this to work.  Can we change the role of the buttons from "AcceptRole" to "ResetRole"?  This style is left-aligned for both Windows and Mac.  I tested that modification and it worked for me.  You can try it with the attached btn.patch.



(file #39015)

Rik <rik5>
Group administrator
Sat 19 Nov 2016 12:28:16 AM UTC, comment #9: 

Right, the order wouldn't change for you because under KDE buttons are places left-to-right, same as under Windows (slightly different order, but with questdlg all 3 buttons are AcceptRole).

Under macOS and GNOME, any buttons with AcceptRole will be placed right-to-left, and will be right-justified.

I think the only way to preserve the default button order for things like "do you want to create this new file?" vs user-created dialogs like questdlg would be to add a new bool to the octave_link call that can differentiate between user-specified buttons and system-specified buttons.

Mike Miller <mtmiller>
Group Member
Sat 19 Nov 2016 12:13:23 AM UTC, comment #8: 

That looks like a good option.  There is also using a proxy style.

http://stackoverflow.com/questions/29485179/how-to-swap-the-cancel-and-ok-button-on-qinputdialog-widget


class MyProxyStyle : public QProxyStyle
{
  public:
    int styleHint(StyleHint hint, const QStyleOption *option = 0,
                  const QWidget *widget = 0, QStyleHintReturn *returnData = 0) const
    {
       if (hint == SH_DialogButtonLayout) {
           //return QDialogButtonBox::GnomeLayout;
           return QDialogButtonBox::MacLayout;
         }
        return QProxyStyle::styleHint(hint, option, widget, returnData);
    }
};


I've tried both options and neither of them are changing the order for me though.


Rik <rik5>
Group administrator
Fri 18 Nov 2016 11:58:04 PM UTC, comment #7: 

This change does work for me with questdlg


diff --git a/libgui/src/dialog.cc b/libgui/src/dialog.cc
--- a/libgui/src/dialog.cc
+++ b/libgui/src/dialog.cc
@@ -136,6 +136,9 @@ MessageDialog::MessageDialog (const QStr
   : QMessageBox (QMessageBox::NoIcon, title.isEmpty () ? " " : title,
                  message, 0, 0)
 {
+  // Override platform-dependent button order
+  setStyleSheet ("* { button-layout: 0 }");
+
   // Create a NonModal message.
   setWindowModality (Qt::NonModal);


But this also affects other dialogs, for example the dialog when editing a file that does not yet exist.

I would think that we want those to agree with the system standard style used for other dialog boxes. So this is also not quite good enough of a change.

Mike Miller <mtmiller>
Group Member
Fri 18 Nov 2016 11:46:28 PM UTC, comment #6: 

I think we might be able to use something like


setStyleSheet ("* { button-layout: 0 }");


in the MessageDialog class to override the default system-dependent button order behavior.

Mike Miller <mtmiller>
Group Member
Fri 18 Nov 2016 11:34:37 PM UTC, comment #5: 

It might also be the Qt version.  I just closed a bug report due to a faulty Qt5.4.2 library.

@Guillaume: Are you using KDE or GNOME or something else?

I'm using KDE, which I now know is doing it differently from Mike's GNOME install.  See this report: http://askubuntu.com/questions/206023/how-do-i-prevent-kde-from-messing-up-my-qt-dialog-button-order

Rik <rik5>
Group administrator
Fri 18 Nov 2016 11:10:19 PM UTC, comment #4: 

I'm afraid it's not that simple. Reopening.

The button order was actually correct for me until this change, and remains correct with 4.2.0.

I suspect that Qt presents pushbuttons in a different order depending on what desktop environment / window manager / OS it detects is running, so that it can behave like the native framework expects it to.

Under GNOME, I expect it presents buttons in reverse order because GNOME Human Interface Guidelines (https://developer.gnome.org/hig/stable/dialogs.html.en) say that the "affirmative" (e.g. OK) button on a dialog should be presented after (to the right of) the Cancel button.

There must be a way to tell Qt to override whatever built-in rules it has about what order buttons should appear in, so that the user is in control rather than the windowing environment.

Mike Miller <mtmiller>
Group Member
Fri 18 Nov 2016 05:16:13 PM UTC, comment #3: 

It works for me, thanks!

Guillaume <gyom>
Fri 18 Nov 2016 05:05:16 PM UTC, comment #2: 

I pushed a change to the development branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/92e7def948c2).  Marking report as Ready for Test.

Rik <rik5>
Group administrator
Fri 18 Nov 2016 04:48:33 PM UTC, comment #1: 

This was annoying to track down but the issue is in libgui/src/dialog.cc. 


  int N = qsbutton.size () < role.size () ? qsbutton.size () : role.size ();
  if (N == 0)
    addButton (QMessageBox::Ok);
  else
    {
      for (int i = N-1; i >= 0; i--)
        {
          // Interpret the button role string, because enumeration
          // QMessageBox::ButtonRole can't be made to pass through a signal.
          QString srole = role.at (i);
          QMessageBox::ButtonRole erole = QMessageBox::InvalidRole;
          if (srole == "YesRole")
            erole = QMessageBox::YesRole;
          else if (srole == "NoRole")
            erole = QMessageBox::NoRole;
          else if (srole == "RejectRole")
            erole = QMessageBox::RejectRole;
          else if (srole == "AcceptRole")
            erole = QMessageBox::AcceptRole;

          QPushButton *pbutton = addButton (qsbutton.at (i), erole);
          if (qsbutton.at (i) == defbutton)
            setDefaultButton (pbutton);
          // Make the last button the button pressed when <esc> key activated.
          if (i == N-1)
            {
// FIXME: Why define and then immediately test value?
#define ACTIVE_ESCAPE 1
#if ACTIVE_ESCAPE
              setEscapeButton (pbutton);
#else
              setEscapeButton (0);
#endif
#undef ACTIVE_ESCAPE
            }
        }
    }


As you can see, the code is iterating over the list in reverse.  It's easy enough to change the for loop to run from smallest to largest.  But this change might need to be repeated for all of the other dialog boxes in dialog.cc as well.


Rik <rik5>
Group administrator
Fri 18 Nov 2016 04:31:23 PM UTC, original submission:  

As mentioned in bug #49643, questdlg displays buttons in reverse order:


questdlg ("Question?", "questdlg", "BTN1", "BTN2", "BTN3", "BTN1")


i.e. BNT3 > BTN2 > BTN1 instead of BTN1 > BTN2 > BTN3 as in Matlab.

Guillaume <gyom>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #39015:  btn.patch added by rik5 (1KiB - application/x-download)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by gyom (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-11-21 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2016-11-19 rik5 Attached File- Added btn.patch, #39015
    2016-11-18 mtmiller StatusFixed Confirmed
        Open/ClosedClosed Open
    2016-11-18 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2016-11-18 rik5 StatusConfirmed Ready For Test
    2016-11-18 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code