Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top Pane and User Button #293

Merged
merged 9 commits into from Apr 12, 2019
Merged

Top Pane and User Button #293

merged 9 commits into from Apr 12, 2019

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Apr 2, 2019

Description

Resolves #279
Resolves #298

Summary of changes

UX Review needed

  • [Behavior] Now, instead of having to click directly on the dropdown icon, a user can click anywhere on the UserButton to open the menu.

  • [Behavior] Instead of hiding the refresh icon when "offline" disable it to show disabled image.

  • [Behavior] Added Static & Active states to the refresh icon, see Workstation Client Content Inventory + Design securedrop-ux#17 (comment)

  • [Behavior] Before error messages were not displayed to the user. Now they show up in the center of the top pane for 10 seconds or until replaced with a new error message.

  • [Styling] Added text styles for Refresh timestamp, journalist username, and sign-in button, see https://app.zeplin.io/project/5c807ea562f734bd2756b243/dashboard

  • [Styling] Added a vertical bar and error icon next to the error status message.

Other changes

  • Added background gradients on the top pane, left pane, and sign-in button as placeholders until final design is finished.

Test Plan

  • Click on journalist name to see drop-down menu pop up
  • Click on drop-down arrow to see drop-down menu pop up
  • When signed out, click on star or delete icons to see 10-second error message centered in top pane
  • Test that error pop hides when a message is cleared because of logging in. To do this, hit the star or delete icons to trigger the error message, then log in before 10 seconds is up to see error pop up go away.
  • When signed in, click on refresh and see active-state refresh icon
  • Sign out and see refresh icon change to the disabled image

A little demo

demo3

icon = QIcon()

pixmap = load_image(normal)
scaled = pixmap.scaled(18, 18, Qt.KeepAspectRatio, transformMode=Qt.SmoothTransformation)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll notice I'm using Qt.SmoothTransformation for smoothing which uses bilinear filtering under the hood. Some of the svgs will need to be replaced because opening them directly in the browser shows that they are small and pixel-y so they will not scale nicely. But for now, the smoothing effect makes it look much better than using FastTransformation or svg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the dropdown-arrow svg is just rendered horribly in my firefox applicaiton on linux as well as mac, so the problem wasn't how the svgs were created and exported in Sketch, but rather how they were being loaded into Qt widgets. After making a change to load svgs into icons using addFile or labels using QSvgWidget they now should look ✨ crispy (not blocky)

QPushButton { border: none; }
QPushButton { border: none; color: #fff }
QStatusBar#activity_status_bar { color: #fff; }
QWidget#error_bar { background-color: #f22b5d }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I got the values of the colors using Gimp's color-sampling tool on images from here: https://projects.invisionapp.com/share/XDRB535NR9Z, which contains wireframes for the error status bar, etc (click through the story). I'm grabbing these colors because they are more up-to-date in invisionapp than they are in zeplin. So this is just to get it to look more like the design we're leaning towards. Eventually we'll code in a better way to store and use a color scheme.

"""
self.refresh.hide()
self.error_status_bar.showMessage(message, duration)
self.error_status_timer.start(duration)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error status widget is timed to popup for the same amount of time as we display the message.

@sssoleileraaa sssoleileraaa force-pushed the issue-279-refreshing branch 3 times, most recently from cd6acc0 to 80ceec1 Compare April 3, 2019 23:59
@sssoleileraaa sssoleileraaa changed the title Issue 279 refreshing Refresh, activity and error messaging, user menu Apr 8, 2019
@sssoleileraaa sssoleileraaa force-pushed the issue-279-refreshing branch 5 times, most recently from 508c243 to 50f567d Compare April 9, 2019 21:29
@sssoleileraaa sssoleileraaa marked this pull request as ready for review April 9, 2019 22:01
@heartsucker
Copy link
Contributor

Clicking sign out fails:

Traceback (most recent call last):
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1282, in _show_or_hide_replybox
    self.reply_box.disable()
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1331, in disable
    self.text_edit.setText(_('You need to log in to send replies.'))
RuntimeError: wrapped C/C++ object of type QTextEdit has been deleted

@heartsucker
Copy link
Contributor

The reply box for typing and sending replies is not present for me.

Screenshot_2019-04-10_14-23-12

I know that Debian / xmonad is not the standard env we are targeting, but I think we will need to consider supporting Debian and macOS for developer agility and to allow outside contributions.

@heartsucker
Copy link
Contributor

heartsucker commented Apr 10, 2019

Attempting to delete a source throws an exception:

Traceback (most recent call last):
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/main.py", line 177, in on_source_changed
    self.show_conversation_for(self.current_source, self.controller.is_authenticated)
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/main.py", line 190, in show_conversation_for
    is_authenticated)
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1264, in __init__
    parent=self)
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1170, in __init__
    self.update_conversation(self.source.collection)
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1184, in update_conversation
    self.add_message(conversation_item)
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1218, in add_message
    self.controller.session.refresh(message)
  File "/home/heartsucker/.local/share/virtualenvs/securedrop-client-GExBtLKv/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 1498, in refresh
    self._expire_state(state, attribute_names)
  File "/home/heartsucker/.local/share/virtualenvs/securedrop-client-GExBtLKv/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 1600, in _expire_state
    self._validate_persistent(state)
  File "/home/heartsucker/.local/share/virtualenvs/securedrop-client-GExBtLKv/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 2047, in _validate_persistent
    state_str(state))
sqlalchemy.exc.InvalidRequestError: Instance '<Message at 0x7f2f28029eb8>' is not persistent within this Session

This may be related to what I suggested in #296.

Copy link
Contributor

@heartsucker heartsucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments addressing concerns I have (2 errors, 1 UI regression). I think this PR may need to be split into smaller ones. There's a lot of requirements to check between those tickets and the diff is quite large.

@sssoleileraaa
Copy link
Contributor Author

Clicking sign out fails:

Traceback (most recent call last):
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1282, in _show_or_hide_replybox
    self.reply_box.disable()
  File "/home/heartsucker/code/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py", line 1331, in disable
    self.text_edit.setText(_('You need to log in to send replies.'))
RuntimeError: wrapped C/C++ object of type QTextEdit has been deleted

This has to do with how reply box was being deleted before. Since deleting the reply box is no longer necessary, removing that code fixed the bug.

@sssoleileraaa
Copy link
Contributor Author

The reply box for typing and sending replies is not present for me.

Can you try now that I removed the old code where we deleted the reply box widget?

@ninavizz
Copy link
Member

Cool. I'll timebox some time for myself on Monday to do more organizing/syleguiding w/in Zeplin, too.

@sssoleileraaa sssoleileraaa force-pushed the issue-279-refreshing branch 2 times, most recently from c945a22 to 7e5e2fd Compare April 10, 2019 19:06
@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Apr 10, 2019

@emkll this is now ready for your eyes :)

Update: See #293 (comment). There is an issue with deleting a source, but that was introduced before this PR. I will look into what we've recently changed in the client. We will need to fix that separately from this PR.

@emkll emkll self-requested a review April 10, 2019 19:41
@sssoleileraaa
Copy link
Contributor Author

Agreed upon to-dos:

* Summarize PR title, clarify description

* Break out reply behavior change

* Clarify test plan

These tasks are done.

@sssoleileraaa sssoleileraaa changed the title Add styling for refresh, activity, error messaging, and user menu with some behavior changes Top Pane and User Menu Apr 10, 2019
@sssoleileraaa sssoleileraaa changed the title Top Pane and User Menu Top Pane and User Button Apr 10, 2019
@sssoleileraaa sssoleileraaa moved this from Under Review to In Development in SecureDrop Team Board Apr 11, 2019
@sssoleileraaa sssoleileraaa moved this from In Development to Under Review in SecureDrop Team Board Apr 11, 2019
Copy link
Contributor

@heartsucker heartsucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some nits (basically just linting) and one place the code could be improved to use Qt best practices (parent/child relationships between widgets). None of these block and all of them can be followed up in other tickets. This PR makes some excellent changes, and I'd rather get them in than nit pick at it until it's "perfect." I'm going to leave it unmerged for now so @emkll can take a look too.

securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
securedrop_client/gui/widgets.py Show resolved Hide resolved
securedrop_client/gui/widgets.py Outdated Show resolved Hide resolved
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @creviera ! I went through the test plan as described, and all functionality works as expected. I have noted two minor UI issues (which might be related to my local dev env/setup rather than the application) that in my eyes shouldn't block merge (see below). If someone else can reproduce these, it might be worth following up on those separately.

  • Click on journalist name to see drop-down menu pop up
  • Click on drop-down arrow to see drop-down menu pop up (:exclamation: see 2) below)
  • When signed out, click on star or delete icons to see 10-second error message centered in top pane (:exclamation: see 1) below)
  • Test that error pop hides when a message is cleared because of logging in. To do this, hit the star or delete icons to trigger the error message, then log in before 10 seconds is up to see error pop up go away.
  • When signed in, click on refresh and see active-state refresh icon
  • Sign out and see refresh icon change to the disabled image

1 - Refresh status is cut off when window is not wide enough (this might be related to the introduction of error_status and the width of my client which was set to 960 pixels wide)

screen

2- Source Dropdown

When clicking on the dropdown menu for the source, the popup that appears when selecting delete source and submissions is clicked is positioned top-left of the client window. All other alerts/popups are generally centered. Moving said popup confirmation also generates segfault (likely unrelated):
I also did get a segfault when moving said alertbox (logs [0], trace[1]):

[0]: Logs (appear unrelated to the crash):

2019-04-11 12:13:27,453 - urllib3.connectionpool:205(_new_conn) DEBUG: Starting new HTTP connection (1): localhost:8081
2019-04-11 12:13:27,492 - urllib3.connectionpool:393(_make_request) DEBUG: http://localhost:8081 "GET /api/v1/sources HTTP/1.1" 200 5209
2019-04-11 12:13:27,494 - urllib3.connectionpool:205(_new_conn) DEBUG: Starting new HTTP connection (1): localhost:8081
2019-04-11 12:13:27,505 - urllib3.connectionpool:393(_make_request) DEBUG: http://localhost:8081 "GET /api/v1/sources/5470c649-47af-484f-bd1c-37fac55ece17/submissions HTTP/1.1" 200 1033
2019-04-11 12:13:27,508 - urllib3.connectionpool:205(_new_conn) DEBUG: Starting new HTTP connection (1): localhost:8081
2019-04-11 12:13:27,524 - urllib3.connectionpool:393(_make_request) DEBUG: http://localhost:8081 "GET /api/v1/sources/52919d87-1264-47c6-a622-71e7e7ef658f/submissions HTTP/1.1" 200 1029
2019-04-11 12:13:27,557 - urllib3.connectionpool:205(_new_conn) DEBUG: Starting new HTTP connection (1): localhost:8081
2019-04-11 12:13:27,568 - urllib3.connectionpool:393(_make_request) DEBUG: http://localhost:8081 "GET /api/v1/replies HTTP/1.1" 200 1957
2019-04-11 12:13:27,568 - securedrop_client.storage:80(get_remote_data) INFO: Fetched 2 remote sources.
2019-04-11 12:13:27,569 - securedrop_client.storage:82(get_remote_data) INFO: Fetched 4 remote submissions.
2019-04-11 12:13:27,569 - securedrop_client.storage:83(get_remote_data) INFO: Fetched 4 remote replies.
2019-04-11 12:13:27,569 - securedrop_client.logic:266(completed_api_call) INFO: Completed API call. Cleaning up and running callback.
2019-04-11 12:13:27,572 - securedrop_client.storage:137(update_sources) DEBUG: Updated source 5470c649-47af-484f-bd1c-37fac55ece17
2019-04-11 12:13:27,575 - securedrop_client.storage:137(update_sources) DEBUG: Updated source 52919d87-1264-47c6-a622-71e7e7ef658f
2019-04-11 12:13:27,589 - securedrop_client.storage:201(__update_submissions) DEBUG: Updated submission 9e16b107-aab1-4c00-a47a-ae96d8bfea5d
2019-04-11 12:13:27,590 - securedrop_client.storage:201(__update_submissions) DEBUG: Updated submission 01d08959-838a-410e-bc23-082441bbfd1b
2019-04-11 12:13:27,591 - securedrop_client.storage:201(__update_submissions) DEBUG: Updated submission d1dca806-b805-4cc9-bd35-778e33c368c8
2019-04-11 12:13:27,593 - securedrop_client.storage:201(__update_submissions) DEBUG: Updated submission 718dba84-3348-410e-9c24-64a280ccb036
2019-04-11 12:13:27,597 - securedrop_client.storage:247(update_replies) DEBUG: Updated reply 28827186-7fc3-4cd9-a0c9-38e5a21d09a9
2019-04-11 12:13:27,599 - securedrop_client.storage:247(update_replies) DEBUG: Updated reply 09054b96-7982-420e-876c-2b6804f6c77f
2019-04-11 12:13:27,600 - securedrop_client.storage:247(update_replies) DEBUG: Updated reply 86a2c43e-799f-4b4b-9ac4-4744e0c4625c
2019-04-11 12:13:27,602 - securedrop_client.storage:247(update_replies) DEBUG: Updated reply cea24e0f-46f3-44b7-b490-dde048300d3c

[1] Trace:

*** Error in `python': double free or corruption (out): 0x00007ffc6b115ab0 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x70bfb)[0x7e599fbebbfb]
/lib/x86_64-linux-gnu/libc.so.6(+0x76fc6)[0x7e599fbf1fc6]
/lib/x86_64-linux-gnu/libc.so.6(+0x7780e)[0x7e599fbf280e]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN14QObjectPrivate14deleteChildrenEv+0x63)[0x7e599c3e2e33]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN7QWidgetD2Ev+0x374)[0x7e599d203a34]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x203ed9)[0x7e599daafed9]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN7QObject5eventEP6QEvent+0xf8)[0x7e599c3e61b8]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN7QWidget5eventEP6QEvent+0x903)[0x7e599d208723]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x26b5e3)[0x7e599db175e3]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent+0x9c)[0x7e599d1cb63c]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN12QApplication6notifyEP7QObjectP6QEvent+0x227)[0x7e599d1d2987]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x36bc9e)[0x7e599dc17c9e]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN16QCoreApplication15notifyInternal2EP7QObjectP6QEvent+0x108)[0x7e599c3ba028]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN23QCoreApplicationPrivate16sendPostedEventsEP7QObjectiP11QThreadData+0x2db)[0x7e599c3bcabb]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(+0x2d3b63)[0x7e599c40eb63]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_dispatch+0x2a7)[0x7e59992307f7]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(+0x4aa60)[0x7e5999230a60]
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_iteration+0x2c)[0x7e5999230b0c]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN20QEventDispatcherGlib13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE+0x5f)[0x7e599c40e1af]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/plugins/platforms/../../lib/libQt5XcbQpa.so.5(+0xd8b81)[0x7e59937ffb81]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN10QEventLoop4execE6QFlagsINS_17ProcessEventsFlagEE+0xea)[0x7e599c3b889a]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN7QDialog4execEv+0x197)[0x7e599d3a6827]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(+0x365cd5)[0x7e599d3d7cd5]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x198257)[0x7e599da44257]
python(PyCFunction_Call+0x77)[0x5e4bd1653287]
python(PyEval_EvalFrameEx+0x48ef)[0x5e4bd161dd0f]
python(PyEval_EvalFrameEx+0x4b64)[0x5e4bd161df84]
python(PyEval_EvalCodeEx+0x12f)[0x5e4bd160b16f]
python(+0x1c20e3)[0x5e4bd16550e3]
python(PyObject_Call+0x47)[0x5e4bd169ee17]
python(+0x12934e)[0x5e4bd15bc34e]
python(PyObject_Call+0x47)[0x5e4bd169ee17]
python(PyEval_CallObjectWithKeywords+0x30)[0x5e4bd1609f30]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/sip.so(+0x133dd)[0x7e59965433dd]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/sip.so(+0x134c2)[0x7e59965434c2]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtSvg.so(+0xab46)[0x7e599423bb46]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtSvg.so(+0xee2b)[0x7e599423fe2b]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN7QWidget5eventEP6QEvent+0x1f8)[0x7e599d208018]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtSvg.so(+0xe8b3)[0x7e599423f8b3]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent+0x9c)[0x7e599d1cb63c]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN12QApplication6notifyEP7QObjectP6QEvent+0xc03)[0x7e599d1d3363]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x36bc9e)[0x7e599dc17c9e]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN16QCoreApplication15notifyInternal2EP7QObjectP6QEvent+0x108)[0x7e599c3ba028]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN19QApplicationPrivate14sendMouseEventEP7QWidgetP11QMouseEventS1_S1_PS1_R8QPointerIS0_Eb+0x1df)[0x7e599d1d1fcf]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(+0x1afb3d)[0x7e599d221b3d]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(+0x1b2453)[0x7e599d224453]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent+0x9c)[0x7e599d1cb63c]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5(_ZN12QApplication6notifyEP7QObjectP6QEvent+0x227)[0x7e599d1d2987]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x36bc9e)[0x7e599dc17c9e]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN16QCoreApplication15notifyInternal2EP7QObjectP6QEvent+0x108)[0x7e599c3ba028]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Gui.so.5(_ZN22QGuiApplicationPrivate17processMouseEventEPN29QWindowSystemInterfacePrivate10MouseEventE+0x370)[0x7e599c9bd300]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Gui.so.5(_ZN22QGuiApplicationPrivate24processWindowSystemEventEPN29QWindowSystemInterfacePrivate17WindowSystemEventE+0x105)[0x7e599c9bf0d5]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Gui.so.5(_ZN22QWindowSystemInterface22sendWindowSystemEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE+0xbb)[0x7e599c99a82b]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/plugins/platforms/../../lib/libQt5XcbQpa.so.5(+0xd8b8b)[0x7e59937ffb8b]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN10QEventLoop4execE6QFlagsINS_17ProcessEventsFlagEE+0xea)[0x7e599c3b889a]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/Qt/lib/libQt5Core.so.5(_ZN16QCoreApplication4execEv+0x84)[0x7e599c3c1424]
/home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/lib/python3.5/site-packages/PyQt5/QtWidgets.so(+0x1f73e0)[0x7e599daa33e0]
python(PyCFunction_Call+0x4f)[0x5e4bd165325f]
python(PyEval_EvalFrameEx+0x48ef)[0x5e4bd161dd0f]
python(PyEval_EvalFrameEx+0x4b64)[0x5e4bd161df84]
python(PyEval_EvalFrameEx+0x4b64)[0x5e4bd161df84]
python(+0x18fb06)[0x5e4bd1622b06]
python(PyEval_EvalCode+0x1f)[0x5e4bd16237bf]
======= Memory map: ========
5e4bd1493000-5e4bd1883000 r-xp 00000000 ca:10 28251                      /home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/bin/python3.5
5e4bd1a82000-5e4bd1a84000 r--p 003ef000 ca:10 28251                      /home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/bin/python3.5
5e4bd1a84000-5e4bd1b1b000 rw-p 003f1000 ca:10 28251                      /home/user/.local/share/virtualenvs/securedrop-client-lniAgW2s/bin/python3.5
5e4bd1b1b000-5e4bd1b4c000 rw-p 00000000 00:00 0 
5e4bd3005000-5e4bd4599000 rw-p 00000000 00:00 0                          [heap]
7e593b7ff000-7e593b800000 ---p 00000000 00:00 0 
7e593b800000-7e593c000000 rw-p 00000000 00:00 0 
7e593c000000-7e593c026000 rw-p 00000000 00:00 0 
7e593c026000-7e5940000000 ---p 00000000 00:00 0 
7e59407f9000-7e59407fa000 ---p 00000000 00:00 0 
7e59407fa000-7e5940ffa000 rw-p 00000000 00:00 0 
7e5940ffa000-7e5940ffb000 ---p 00000000 00:00 0 
7e5940ffb000-7e59417fb000 rw-p 00000000 00:00 0 
7e59417fb000-7e59417fc000 ---p 00000000 00:00 0 
7e59417fc000-7e5941ffc000 rw-p 00000000 00:00 0 
7e5941ffc000-7e5941ffd000 ---p 00000000 00:00 0 
7e5941ffd000-7e59427fd000 rw-p 00000000 00:00 0 
7e59427fd000-7e59427fe000 ---p 00000000 00:00 0 
7e59427fe000-7e5942ffe000 rw-p 00000000 00:00 0 
7e5942ffe000-7e5942fff000 ---p 00000000 00:00 0 
7e5942fff000-7e59437ff000 rw-p 00000000 00:00 0 
7e59437ff000-7e5943800000 ---p 00000000 00:00 0 
7e5943800000-7e5944000000 rw-p 00000000 00:00 0 
7e5944000000-7e5944021000 rw-p 00000000 00:00 0 
7e5944021000-7e5948000000 ---p 00000000 00:00 0 
7e5948000000-7e5948025000 rw-p 00000000 00:00 0 
7e5948025000-7e594c000000 ---p 00000000 00:00 0 
7e594c000000-7e594c024000 rw-p 00000000 00:00 0 
7e594c024000-7e5950000000 ---p 00000000 00:00 0 
7e5950000000-7e5950022000 rw-p 00000000 00:00 0 
7e5950022000-7e5954000000 ---p 00000000 00:00 0 
7e5954000000-7e5954021000 rw-p 00000000 00:00 0 
7e5954021000-7e5958000000 ---p 00000000 00:00 0 
7e5958000000-7e5958024000 rw-p 00000000 00:00 0 
7e5958024000-7e595c000000 ---p 00000000 00:00 0 
7e595c000000-7e595c107000 rw-p 00000000 00:00 0 
7e595c107000-7e5960000000 ---p 00000000 00:00 0 
7e5960000000-7e5960021000 rw-p 00000000 00:00 0 
7e5960021000-7e5964000000 ---p 00000000 00:00 0 
7e5964000000-7e5964104000 rw-p 00000000 00:00 0 
7e5964104000-7e5968000000 ---p 00000000 00:00 0 
7e59687f9000-7e59687fa000 ---p 00000000 00:00 0 
7e59687fa000-7e5968ffa000 rw-p 00000000 00:00 0 
7e5968ffa000-7e5968ffb000 ---p 00000000 00:00 0 
7e5968ffb000-7e59697fb000 rw-p 00000000 00:00 0 
7e59697fb000-7e59697fc000 ---p 00000000 00:00 0 
7e59697fc000-7e5969ffc000 rw-p 00000000 00:00 0 
7e5969ffc000-7e5969ffd000 ---p 00000000 00:00 0 
7e5969ffd000-7e596a7fd000 rw-p 00000000 00:00 0 
7e596a7fd000-7e596a7fe000 ---p 00000000 00:00 0 
7e596a7fe000-7e596affe000 rw-p 00000000 00:00 0 
7e596affe000-7e596afff000 ---p 00000000 00:00 0 
7e596afff000-7e596b7ff000 rw-p 00000000 00:00 0 
7e596b7ff000-7e596b800000 ---p 00000000 00:00 0 
7e596b800000-7e596c000000 rw-p 00000000 00:00 0 
7e596c000000-7e596c021000 rw-p 00000000 00:00 0 
7e596c021000-7e5970000000 ---p 00000000 00:00 0 
7e59704ea000-7e59704eb000 ---p 00000000 00:00 0 
7e59704eb000-7e5970ceb000 rw-p 00000000 00:00 0 
7e5970ceb000-7e5970cec000 ---p 00000000 00:00 0 
7e5970cec000-7e59714ec000 rw-p 00000000 00:00 0 
7e59714ec000-7e59714ed000 ---p 00000000 00:00 0 
7e59714ed000-7e5971ced000 rw-p 00000000 00:00 0 
7e5971d5c000-7e5972128000 rw-s 00000000 00:05 3637254                    /SYSV00000000 (deleted)
7e5972128000-7e5972132000 r-xp 00000000 ca:03 291040                     /lib/x86_64-linux-gnu/libnss_files-2.24.so
7e5972132000-7e5972332000 ---p 0000a000 ca:03 291040                     /lib/x86_64-linux-gnu/libnss_files-2.24.so
7e5972332000-7e5972333000 r--p 0000a000 ca:03 291040                     /lib/x86_64-linux-gnu/libnss_files-2.24.so
7e5972333000-7e5972334000 rw-p 0000b000 ca:03 291040                     /lib/x86_64-linux-gnu/libnss_files-2.24.so
7e5972334000-7e597233a000 rw-p 00000000 00:00 0 
7e5972581000-7e59725c1000 rw-p 00000000 00:00 0 
7e59725c1000-7e597266e000 r--p 00000000 ca:03 521567                     /usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf
7e597266e000-7e5972771000 r-xp 00000000 ca:03 267854                     /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6
7e5972771000-7e5972970000 ---p 00103000 ca:03 267854                     /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6
7e5972970000-7e5972973000 r--p 00102000 ca:03 267854                     /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6
7e5972973000-7e5972975000 rw-p 00105000 ca:03 267854                     /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6
7e5972975000-7e5972976000 rw-p 00000000 00:00 0 
7e5972976000-7e597298a000 r-xp 00000000 ca:03 398963                     /usr/lib/python3.5/lib-dynload/_sqlite3.cpython-35m-x86_64-linux-gnu.so
7e597298a000-7e5972b8a000 ---p 00014000 ca:03 398963                     /usr/lib/python3.5/lib-dynload/_sqlite3.cpython-35m-x86_64-linux-gnu.so
7e5972b8a000-7e5972b8b000 r--p 00014000 ca:03 398963                     /usr/lib/python3.5/lib-dynload/_sqlite3.cpython-35m-x86_64-linux-gnu.so
7e5972b8b000-7e5972b8e000 rw-p 00015000 ca:03 398963                     /usr/lib/python3.5/lib-dynload/_sqlite3.cpython-35m-x86_64-linux-gnu.so
7e5972b8e000-7e5972bc6000 r-xp 00000000 ca:03 280993                     /usr/lib/x86_64-linux-gnu/s2tc/libtxc_dxtn.so
7e5972bc6000-7e5972dc6000 ---p 00038000 ca:03 280993                     /usr/lib/x86_64-linux-gnu/s2tc/libtxc_dxtn.so
7e5972dc6000-7e5972dc7000 r--p 00038000 ca:03 280993                     /usr/lib/x86_64-linux-gnu/s2tc/libtxc_dxtn.so
7e5972dc7000-7e5972dc8000 rw-p 00039000 ca:03 280993                     /usr/lib/x86_64-linux-gnu/s2tc/libtxc_dxtn.so
7e5972dc8000-7e5972dc9000 ---p 00000000 00:00 0 
7e5972dc9000-7e59735c9000 rw-p 00000000 00:00 0 
7e59735c9000-7e59735ca000 ---p 00000000 00:00 0 
7e59735ca000-7e5973dca000 rw-p 00000000 00:00 0 
7e5973dca000-7e5973dec000 r-xp 00000000 ca:03 264579                     /lib/x86_64-linux-gnu/libncurses.so.5.9
7e5973dec000-7e5973feb000 ---p 00022000 ca:03 264579                     /lib/x86_64-linux-gnu/libncurses.so.5.9./run.sh: line 44: 14345 Aborted                 exec python -m securedrop_client --sdc-home "$SDC_HOME" --no-proxy $@

securedrop_client/gui/widgets.py Show resolved Hide resolved
@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Apr 11, 2019

Thanks for the great feedback @emkll! Here's my followup:

@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Apr 11, 2019

@heartsucker thanks for the final review. I think I addressed all of mickael's and your concerns. See all the Issues I created and linked to in this PR to keep the scope of this PR more narrow. Lemme know if I missed something!

I wonder if we should wait before merging any more PRs until the bugs on master are fixed? This PR shouldn't affect any of them like the SourceMenu segfault or segfault when deleting a Source (since this pr is related only to the TopPane and UserButton) but they did enter into the conversation during the review of this PR. And creating a Test Plan and wiki and all the Issues for things mentioned here outside of the scope of this PR took quite a bit of time.

If we're not confident about the client as is, let's spend some time to fix things. @eloquence thoughts?

@eloquence
Copy link
Member

How about this to get some sanity back:

Would that make sense to you @creviera @redshiftzero ? It's only ~2 1/2 more days in the sprint so we're not talking about a huge delay.

@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Apr 11, 2019

I have some nits (basically just linting) and one place the code could be improved to use Qt best practices (parent/child relationships between widgets). None of these block and all of them can be followed up in other tickets. This PR makes some excellent changes, and I'd rather get them in than nit pick at it until it's "perfect." I'm going to leave it unmerged for now so @emkll can take a look too.

I created a place for us to discuss parent child relationships of our widgets, and also related, separation of concerns. For example, once a child has a parent, it can call methods on its parent, which makes it hard to reason about what each widget is responsible for. Often it looks like we pass parents to make it easier for children to tell the parent that they would like the parent, or its parent, or its parent, etc., to do a database operation. Anyway, I think this is a bigger discussion than how to design how we hide or show widgets. So go here to get opinionated 💃 🎉 #303

@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Apr 11, 2019

* prioritize segfaults/core functionality breakage (#302 and #297, any others)?

One other that I'm working on investigating and writing up a ticket for. I may have to open the issue without having an STR that consistently produces the segfault.

* bump most other styling/feature work off this sprint except maybe the small-ish #286

Makes sense to me. I like the idea of figuring digging into logic.py more (good cross-team learning opportunity).

@redshiftzero
Copy link
Contributor

yep let's merge this and then focus on resolving bugs

@redshiftzero redshiftzero merged commit 8f69f01 into master Apr 12, 2019
SecureDrop Team Board automation moved this from Under Review to Done Apr 12, 2019
@redshiftzero redshiftzero deleted the issue-279-refreshing branch April 12, 2019 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
6 participants