Add a menu option and dialog to verify a signed message #906

sje397 commented at 11:58 AM on February 27, 2012: contributor

Being able to sign a message without the ability to verify signed messages is a little lopsided :)

luke-jr commented at 10:34 PM on February 27, 2012: member

This doesn't respect FIRST_CLASS_MESSAGING=1
Instead of a new dialog/tab, perhaps it would make sense to just add a button to the Sign Message one (and rename it Messages or something)?

sje397 commented at 11:13 PM on February 27, 2012: contributor

I tried modifying the existing window initially. It was ugly. The address field should be below the message when verifying, and having lots of hiding/showing components feels like a design problem to me - too much responsibility for a single UI class that really should be separated.

I treated 'FIRST_CLASS_MESSAGING' as relating to signing messages only. As per above, I think the UI should be separate and I don't think subtabs make sense in a first-class messaging window...and I don't think having two tabs for messaging in the bitcoin UI is a fair waiting given the main purpose. Basically, I don't think 'FIRST_CLASS_MESSAGING' makes any sense :)

luke-jr commented at 11:21 PM on February 27, 2012: member

How about moving the message to the top, then have the address and signature under that, with "Sign" and "Verify" buttons between the addr and sig textlines, with arrows of which direction it processes?

sje397 commented at 2:48 AM on February 28, 2012: contributor

That sounds pretty good to me. I'll mess around with it this evening.

sje397 commented at 11:13 AM on February 28, 2012: contributor

I still don't like it. It means having the signature line edit writeable when you may just want to sign and have that generated...and it means having the address book and paste buttons enabled when you might just want to validate. It also means the order of elements on the page is not necessarily what you want to see (i.e. to verify, you fill in message, then skip address, then fill in signature). Plus there are other ugly issues with the way things are done now - e.g. select one of your own addresses, click sign message, click address book (thereby opening a window containing the same list of addresses you just left) and there is a sign message button again, which does nothing perceptible.

I would rather leave the message functionality in a menu somewhere (maybe the same should be true of the qrcode generation function) so that it doesn't bother users that just want the basics, and keep the sign and validate UI elements simple.

luke-jr commented at 2:14 AM on March 9, 2012: member

While validation is more of an advanced tool, signing is basics. Perhaps a UI expert should provide input on how to best integrate them together.

laanwj commented at 6:33 PM on March 10, 2012: member

Good, indeed it was asymmetric to have signing without verifying, this at least makes it complete.

IMO you don't have to do any extra work to make it play nice with FIRST_CLASS_MESSAGING. Only @luke-jr uses that option, let him do that :)

TheBlueMatt commented at 4:39 PM on March 12, 2012: member

Why do we have a FIRST_CLASS_MESSAGING if no one uses it, also, it needs a much more descriptive name, FIRST_CLASS_MESSAGING doesnt say anything about what it actually does.

luke-jr commented at 4:43 PM on March 12, 2012: member

"No one" uses 0.6 yet. It makes messaging first-class.

laanwj commented at 5:11 PM on March 12, 2012: member

@TheBlueMatt we have FIRST_CLASS_MESSAGING because it was part of the signmessage pull. Luke-jr wanted to add signing messages as a separate tab, whereas according to us there are already too many tabs and we should reserve them for essential functionality. So he put it behind a qmake option, I guess because he wants to keep it as a tab himself...

TheBlueMatt commented at 5:15 PM on March 12, 2012: member

Mmm, so maybe it should be SIGNMESSAGE_IN_TAB or something that actually describes what it does?

luke-jr commented at 6:00 PM on March 12, 2012: member

BTW: If Samuel's GUI gets merged, the "too many tabs" excuse goes away, as there's plenty of room.

laanwj commented at 6:31 PM on March 12, 2012: member

It's not about room (if you change the toolbars to icon-only you also have enough space). It's about not confusing users.

luke-jr commented at 1:44 PM on April 10, 2012: member

Rebasing required.

sje397 commented at 2:27 PM on April 10, 2012: contributor

Rebased.

Diapolo commented at 3:07 PM on April 22, 2012: none

Can anyone post a screenshot, I would like to know where the button is loctated and how it looks currently?

sje397 commented at 3:27 PM on April 22, 2012: contributor

I rebased and fixed a tiny issue due to a change in the CDataStream constructor. I also removed that HTML.

I'm trying to get a screen shot but wrestling with a new install of arch.

laanwj commented at 7:24 AM on May 5, 2012: member

Great to have this, isn't symmetry a beautiful thing.

One suggestion: Please make this non-modal, like the other utility windows. (that'll also make testing easier as we can keep open a "sign message" and "verify message" window open at the same time)

Diapolo commented at 9:18 AM on May 5, 2012: none

I would like to know, where in the menu this is placed, as I would suggest to have it below Sign message (in file) or in a to add tools menu.

sje397 commented at 1:44 PM on May 11, 2012: contributor

The menu option is underneath 'Sign Message' in File, yes. I couldn't get a screenshot because the menu being shown disabled my print-screen key :(

I've rebased, and made the dialog non-modal. I also moved the call to 'setAttribute(Qt::WA_DeleteOnClose)' out of the QRCodeDialog to match how it is handled here. This is better since that decision should be up to the creator of the dialog rather than the dialog itself.

in src/qt/forms/verifymessagedialog.ui:None in 6cef54159e outdated

  31 | +    <widget class="QTextEdit" name="edMessage">
  32 | +     <property name="documentTitle">
  33 | +      <string extracomment="testing"/>
  34 | +     </property>
  35 | +     <property name="html">
  36 | +      <string>&lt;!DOCTYPE HTML PUBLIC &quot;-//W3C//DTD HTML 4.0//EN&quot; &quot;http://www.w3.org/TR/REC-html40/strict.dtd&quot;&gt;

Diapolo commented at 1:47 PM on May 11, 2012:

And once more we have HTML :D.

sje397 commented at 1:55 PM on May 11, 2012:

I tried removing that, but the editor just puts it back in. I'll try hand-editing and see what happens...but I'd like to avoid a situation where people have to manually edit the file if they touch it.

Diapolo commented at 1:58 PM on May 11, 2012:

Can you set this to plainText? Perhaps this helps? Or can you use another GUI element that does the same? The messagepage uses an QPlainTextEdit field and I would prefer them to be even in terms of GUI element usage.

in src/qt/forms/verifymessagedialog.ui:None in 6cef54159e outdated

  28 | +    </widget>
  29 | +   </item>
  30 | +   <item>
  31 | +    <widget class="QTextEdit" name="edMessage">
  32 | +     <property name="documentTitle">
  33 | +      <string extracomment="testing"/>

Diapolo commented at 1:47 PM on May 11, 2012:

What's an extracomment?

in src/qt/forms/verifymessagedialog.ui:None in 6cef54159e outdated

  43 | +   </item>
  44 | +   <item>
  45 | +    <widget class="QLineEdit" name="lnSig">
  46 | +     <property name="font">
  47 | +      <font>
  48 | +       <italic>false</italic>

Diapolo commented at 1:48 PM on May 11, 2012:

Guess italic false is default, so this can dissappear ... same for a few lines below.

sje397 commented at 2:03 PM on May 11, 2012: contributor

Changed to QPlainTextEdit (on Diapolo's advice), found the button I'd lost to reset the checkboxes so the 'italic false' stuff should be gone too.

Diapolo commented at 2:06 PM on May 11, 2012: none

You have to excuse me, as I'm kind of a perfectionist ^^, I even care about the smallest things ... sometimes this is bad, but more often it's a good thing :D.

sje397 commented at 2:09 PM on May 11, 2012: contributor

No, that's a very good thing. I'd rather end up with better software too :)

Add a menu option to verify a signed message

(Also move 'setAttribute(Qt::WA_DeleteOnClose)' out of QRCodeDialog)

0c587936c7

in src/qt/forms/verifymessagedialog.ui:None in f032ce8097 outdated

  69 | +       </property>
  70 | +       <property name="toolTip">
  71 | +        <string>Copy the currently selected address to the system clipboard</string>
  72 | +       </property>
  73 | +       <property name="text">
  74 | +        <string>&amp;Copy to Clipboard</string>

Diapolo commented at 2:11 PM on May 11, 2012:

If this copies the resulting address, please use copy "Copy Address" as text.

laanwj referenced this in commit bb361cc644 on May 14, 2012

laanwj merged this on May 14, 2012

laanwj closed this on May 14, 2012

coblee referenced this in commit 38fab7ce1f on Jul 17, 2012

suprnurd referenced this in commit 113e56dd8a on Dec 5, 2017

ptschip referenced this in commit 62d36dd43f on Jan 17, 2018

lateminer referenced this in commit 8a47747b59 on Oct 30, 2019

sipa referenced this in commit bdca9bcb6c on Apr 23, 2021

rebroad referenced this in commit 6e13b4e771 on Jun 23, 2021

DrahtBot locked this on Sep 8, 2021