1219887 - [gui] Log view panel collapses when I minimize the application

Status: RESOLVED FIXED

(Testing :: mozregression, defect)

Description

[arni2033]

> Video:   https://dl.dropboxusercontent.com/s/dg2sgq21hgucz4c/video%20-%20mozregression%20%5Bgui%5D%20Log%20view%20panel%20collapses%20when%20I%20minimize%20the%20application.webm?dl=0

STR:   (Win7_64, ver. 0.4.3, using mozregression v. 1.0.1)
1. Open mozregression GUI, start new bisection (or cancel it)
2. Click "Views", click "Log view" to enable it
3. Minimize the application
4. Click the application button on the taskbar to focus the window (or Alt-Tab to the window)

Result:       There's no Log view (I can't say wheather it should work like this - leaving Unconfirmed)
Expectations: Log view should stay

BTW:          Thanks for working on GUI version; I'll upload 1-2 more bugs when I get more time.

Comment 1

[Julien Pagès (:parkouss)]

Thanks for the bug report - I'm glad that you use mozregression gui, and bugs are welcome to improve it!

So no, it should not work like this - though I can't reproduce on my Linux box - will give it a try on windows later.

Comment 2

[MikeLing]

hmm,I just test it on my Mac OS X and it works fine to me.

Comment 3

[Julien Pagès (:parkouss)]

So yeah, this is a windows specific issue. I can reproduce with latest pyqt on win32 at least.

There is a workaround stated here:

http://stackoverflow.com/questions/4633826/qdockwidget-is-closed-if-main-window-is-minimized

Let's see if someone is interested to fix that as a first good bug (or just fix that!). Else I'll look into it later.

Comment 4

[Wasif Hyder]

(In reply to Julien Pagès (:parkouss) from comment #3)
> So yeah, this is a windows specific issue. I can reproduce with latest pyqt
> on win32 at least.
> 
> There is a workaround stated here:
> 
> http://stackoverflow.com/questions/4633826/qdockwidget-is-closed-if-main-
> window-is-minimized
> 
> Let's see if someone is interested to fix that as a first good bug (or just
> fix that!). Else I'll look into it later.

Hi, new Mozilla-ian reporting for duty. I'd love to work on this bug, could you elaborate a bit on it?

Comment 5

[Julien Pagès (:parkouss)]

Hi wasif,

Sure, it's cool that you are interested to work on this bug!

So, this is all pretty well explained in  comment 0 - if you follow the steps you should see the bug (and there is the excellent video, thanks to arni2033!).

Basically we have a "log view" that you can show/hide from the Edit menu, and on windows if you make it visible then minimize the application, the log view will be hidden automatically (and we don't want that;).

So, first step would be for you to be able to run the application, and reproduce the issue! You should clone the mozregression repo [1] and follow the setup development in [2]. That should help you to get started. After that, you can look at the stackoverflow link I gave, or try to find for another solution - this seems to be a well known Qt bug (we use Qt - and so PyQt as our GUI framework, [3] and [4])

I assign you the bug, please ask if you have questions. :)

[1] https://github.com/mozilla/mozregression
[2] https://github.com/mozilla/mozregression/blob/master/gui/README.rst
[3] http://www.qt.io/
[4] https://riverbankcomputing.com/software/pyqt/intro

Comment 6

[Wasif Hyder]

Hi Julien,

Thanks for assigning me on this.

I reproduced the bug. Can you guide me on how to step through the code?

Comment 7

[Wasif Hyder]

(In reply to Julien Pagès (:parkouss) from comment #5)
> Hi wasif,
> 
> Sure, it's cool that you are interested to work on this bug!
> 
> So, this is all pretty well explained in  comment 0 - if you follow the
> steps you should see the bug (and there is the excellent video, thanks to
> arni2033!).
> 
> Basically we have a "log view" that you can show/hide from the Edit menu,
> and on windows if you make it visible then minimize the application, the log
> view will be hidden automatically (and we don't want that;).
> 
> So, first step would be for you to be able to run the application, and
> reproduce the issue! You should clone the mozregression repo [1] and follow
> the setup development in [2]. That should help you to get started. After
> that, you can look at the stackoverflow link I gave, or try to find for
> another solution - this seems to be a well known Qt bug (we use Qt - and so
> PyQt as our GUI framework, [3] and [4])
> 
> I assign you the bug, please ask if you have questions. :)
> 
> [1] https://github.com/mozilla/mozregression
> [2] https://github.com/mozilla/mozregression/blob/master/gui/README.rst
> [3] http://www.qt.io/
> [4] https://riverbankcomputing.com/software/pyqt/intro

Hi Julien,

Thanks for assigning me on this.

I reproduced the bug. Can you guide me on how to step through the code?

Comment 8

[Julien Pagès (:parkouss)]

Great, I'm glad you're able to reproduce it;

Basically all the code related the the GUI is under the gui/mozregui folder:
https://github.com/mozilla/mozregression/tree/master/gui/mozregui

The main window, which I believe is where you need to look at for this bug, is in the gui/mozregui/main.py file:
https://github.com/mozilla/mozregression/blob/master/gui/mozregui/main.py#L47

So this is a QMainWindow instance - the full documentation can be found here:
http://doc.qt.io/qt-4.8/qmainwindow.html

Even if it is a C++ documentation, you should be able to translate easily to Python code. You can also look at the pyqt documentation instead. Note that QMainWindow inherits from the QWidget class, so have a look at that too.

I would start by looking in these classes to somehow find a way to react to hide/show events,  andrespectively save and restore the state of the main window.

Comment 9

[Wasif Hyder]

Hi Julien,

I've narrowed down the bug to this line of code, commenting it out seems to resolve the problem and I can't notice any adverse effects. 

I've attached in a demonstration. 

What do you think?

https://www.dropbox.com/s/4h9rm8k94dl7wjy/moz-bug.gif?dl=0

Comment 10

[Julien Pagès (:parkouss)]

Hey Wasif, great finding!

So, the file you are looking at is a generated file. Here is the file that generates it (and pointing to the problem):

https://github.com/mozilla/mozregression/blob/master/gui/mozregui/ui/mainwindow.ui#L419

Note you should use the qt "designer" application (or qtcreator) to edit these .ui files. It will be much more easier. :)

So, I narrowed down, the issue probably comes from this commit:
https://github.com/mozilla/mozregression/commit/3e48146

It was introduced so the state of the checkboxes in the "Views" menu are correct (even when we close the dock widget logview using its right cross). So I believe if we just remove the visibilityChanged signal, the views menu won't be up to date (to be verified).

So, we have something like this in term of signals/slots:

source            signal                 target         slot
logDockWidget visibilityChanged(bool) actionLogView setChecked(bool)
actionLogView toggled(bool)           logDockWidget setVisible(bool)

Which basically means: when the visibility of the log view changes, check or uncheck the corresponding action (menu Views -> Log View).
And when the action is checked or unchecked, change the visibility. So, for me on windows Qt triggers a visible event when the application is minimized - and it does not do that on Linux, nor mac.

It is indicated here (http://stackoverflow.com/a/5236654) that using triggered(bool) instead of toggled(bool) fix the issue. Maybe you can test that ?

As I said this is a great finding. Thanks for digging this Wasif!

Comment 11

[Wasif Hyder]

Hi Julien, sorry for the delay. 

I used the Qt Designer, and changing the slot to toggled resolves the issue. Should I submit the patch now?

Comment 12

[Wasif Hyder]

* Correction: I changed the slot to triggered*

Comment 13

[Julien Pagès (:parkouss)]

(In reply to wasif_hyder from comment #11)
> Hi Julien, sorry for the delay. 

Hi Wasif!

> I used the Qt Designer, and changing the slot to toggled resolves the issue.
> Should I submit the patch now?

Yes, a patch would be great then! Please create a github Pull request then ask on this bug for a review (you have to select the "Add an attachment" link, then copy paste the PR url in the content field, choose "?" for the review flag and copy my email in the flag then click to send the attachment)

Yeah, sounds complex, but it is not once you have made it a first time! This is all detailled in http://ateam-bootcamp.readthedocs.org/en/latest/guide/development_process.html#git-and-github, please read ti carefully.

Thanks!

Comment 14

[Wasif Hyder]

Attachment: Bugfix - actionLogView disappears on minimizing

Comment 15

[Julien Pagès (:parkouss)]

Comment on attachment 8702480 [details] [review]
Bugfix - actionLogView disappears on minimizing

Thanks! Looks good to me, and it works well. :)

Comment 16

[Julien Pagès (:parkouss)]

Landed in https://github.com/mozilla/mozregression/commit/fea2cf216c1aa0293f62009d54405567ad91d8fa.

Wasif, if you are interested to tackle other bugs, please do! We can find some bugs together in the next days (I'm a bit busy with the end of the year parties :)) - or you can have a look at mozregression bugs and start working / or add comments on bug that interest you.

I'm often on mozilla irc, in the #ateam channel and my nickname is parkouss - we can chat here if you want (also in some days).

Thanks again for resolving this!

Comment 17

[Julien Pagès (:parkouss)]

Oh, I have a bug in mind finally. Wasif, you can have a look at bug 1173984 if you are interested. It's an interesting bug to work on - read carefully the comments, and if you are interested and feel that you can work on it, please add a comment in the bug to say so. :)

Comment 18

[Wasif Hyder]

(In reply to Julien Pagès (:parkouss) from comment #17)
> Oh, I have a bug in mind finally. Wasif, you can have a look at bug 1173984
> if you are interested. It's an interesting bug to work on - read carefully
> the comments, and if you are interested and feel that you can work on it,
> please add a comment in the bug to say so. :)

Hey, yeah! I'd love to work on it. I want to get involved with this project, and have been reading through the code in my free time to understand it.

I'll look into it and add a comment!

Enjoy the new years parties, and thanks so much!

Comment 19

[Wasif Hyder]

https://github.com/mozilla/mozregression/pull/399

Done. I've implemented it as checkable, but there are some issues. Kindly advise.