Project

General

Profile

Bug #15894

Feature #14568: Additional Software Packages

Persistence configuration opens on full screen

Added by sajolida over 1 year ago. Updated 12 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Persistence
Target version:
Start date:
09/02/2018
Due date:
% Done:

100%

Feature Branch:
persistence-setup:bugfix/15894-full-screen-tps
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

I tried to create a Persistence on Tails 3.9 (20180817) and got the first screen to open in full size of my usual screen. Before it used to open on a smaller window that was easier to read. Is this related to the changes done for Additional Software?

small-persistence-setup.png View (575 KB) sajolida, 09/10/2018 07:19 PM


Related issues

Related to Tails - Bug #16311: Test suite: persistence.feature vs. buster, notifications… In Progress 01/06/2019

Associated revisions

Revision aff3a37b (diff)
Added by intrigeri about 1 year ago

Enable the bugfix-15894-full-screen-tps APT overlay (refs: #15894).

Revision d78fd6ad
Added by intrigeri about 1 year ago

Merge branch 'bugfix/15894-full-screen-tps' into stable (Fix-committed: #15894)

History

#1 Updated by intrigeri over 1 year ago

  • Assignee changed from intrigeri to sajolida
  • QA Check set to Info Needed

Is this related to the changes done for Additional Software?

Yes, most likely.

How would you rank this wrt. benefit in the post-user-testing UX evaluation?

#2 Updated by sajolida over 1 year ago

  • Status changed from New to Confirmed
  • Assignee changed from sajolida to intrigeri

If that helps debugging, I checked my recordings and this was not affecting the ISO that I tested with users back in May. Otherwise I think I would have reported it. So it's a regression both on what we have in 3.8 and what we had in the feature branch when I tested it.

I haven't seen people try to use it so I can't be sure how much it hurts usability but it looks buggy and probably hurts readability and slows down users. That would also get worse for people on desktops with larger screen.

Double-clicking on the title bar to get out of full screen doesn't bring back the window to its original but only makes it slightly smaller (on my display).

#3 Updated by intrigeri over 1 year ago

  • Assignee changed from intrigeri to sajolida

If that helps debugging, I checked my recordings and this was not affecting the ISO that I tested with users back in May. Otherwise I think I would have reported it. So it's a regression both on what we have in 3.8 and what we had in the feature branch when I tested it.

Interesting. Since May the only change applies to t-p-s that can remotely be related to this problem is the fix for #15784 i.e. https://git-tails.immerda.ch/persistence-setup/commit/?id=0b3d39df0d674aa0cc511e7cf15e0cd2d929a50c. Reverting it does not fix the problem for me. So I really don't see how this bug can possibly have appeared since May :/ But of course I trust your recordings so I've tested some more (see below).

Two questions for you:

  • Do you still have the full filename of the ISO you've tested back then? It should have commit IDs which will help me debug this.
  • Can you tell me what screen resolution(s) was used during the May testing session?

I haven't seen people try to use it so I can't be sure how much it hurts usability but it looks buggy and probably hurts readability and slows down users. That would also get worse for people on desktops with larger screen.

OK, thank you for the analysis.

I cannot reproduce the bug with:

  • a 960×942 display, i.e. what I use most of the time for testing
  • a WXGA+ (1440×900) display, still common on some 13" laptops
  • a Full HD (1920×1080) display, the most common resolution since ~5 years, so "That would also get worse for people on desktops with larger screen" is probably wrong.

In both cases the window size is 457×516, as requested by the app.

But I can reproduce it with:

  • 1280×800 display
  • HD (1366×768), still common on some cheap 11" laptops

So I'd like to amend your analysis to make it clear that only some screen resolutions are affected, and those screen resolutions are only common on older laptops: since 5+ years most laptops have WXGA+ (1440×900) or FHD (1920×1080). If most of your recordings were made with one of these resolutions, that are not affected by the bug, this would explain why this looks like a regression that appeared since May (and would confirm my hunch wrt. which resolutions are most common, which in turn would affect how important this bug is).

Double-clicking on the title bar to get out of full screen doesn't bring back the window to its original but only makes it slightly smaller (on my display).

I've tested this on a 1280×800 display (I guess that's what you were using when noticing this bug) and could reproduce it. Clicking the "not full screen" icon in the title bar makes the window quite smaller (~1130×500 instead of ~1280×700). Maybe that's what you meant with "slightly smaller".

This behaviour would be explained by this hypothesis:

  1. t-p-s requests some size that's close to fullscreen (that's probably the cause of the bug)
  2. GNOME Shell realizes it's almost fullscreen and sets the window to fullscreen (I've seen the same happen for Tor Browser, did not check the source code but this would make sense to me)

But this seems very surprising to me since in my test, the non-fullscreen window size is quite far from fullscreen so I doubt step 2 should happen.

Also, note to myself — in all cases xprop tells me:

WM_NORMAL_HINTS(WM_SIZE_HINTS):
        program specified minimum size: 457 by 516
        program specified base size: 0 by 0

… which seems correct. So the bug definitely seems to be on the GNOME Shell side.

#4 Updated by sajolida about 1 year ago

  • Do you still have the full filename of the ISO you've tested back then? It should have commit IDs which will help me debug this.

I tested with f25dce19c6.

See https://mailman.boum.org/pipermail/tails-ux/2018-May/003543.html.

  • Can you tell me what screen resolution(s) was used during the May testing session?

1280x800. I did the test on my spare X200.

But I can reproduce it with:

  • 1280×800 display

That's what I have.

#5 Updated by sajolida about 1 year ago

Attaching a screenshot of how it looked in f25dce19c6. The user opened it from the link in Additional Software with no persistence created.

#6 Updated by intrigeri about 1 year ago

FWIW help desk has not heard about this problem. Does not mean it doesn't harm usability, of course.

#7 Updated by intrigeri about 1 year ago

  • Status changed from Confirmed to In Progress
  • Assignee changed from intrigeri to CyrilBrulebois
  • Target version set to Tails_3.10.1
  • % Done changed from 0 to 50
  • QA Check changed from Info Needed to Ready for QA
  • Feature Branch set to persistence-setup:bugfix/15894-full-screen-tps
  • Type of work changed from Test to Code

sajolida wrote:

  • Do you still have the full filename of the ISO you've tested back then? It should have commit IDs which will help me debug this.

I tested with f25dce19c6.

Great! That's all I needed to bisect and identify the faulty commit: reverting
https://git-tails.immerda.ch/persistence-setup/commit/?id=131749e897666bd9f10b6436e5b57781f0db1829 fixes the problem. I have no idea why but I've implemented a fix which works for me on various resolutions including 1280×800.

kibi, could you please take a quick look at https://git-tails.immerda.ch/persistence-setup/commit/?h=bugfix/15894-full-screen-tps, confirm my one-liner fix is not doing something very stupid or dangerous, and if happy I'll merge and it'll be part of 3.10. Don't bother testing. If you want to take a look at the GTK doc, start Devhelp and search for gtk_label_set_max_width_char.

#8 Updated by intrigeri about 1 year ago

  • Parent task set to #14568

#9 Updated by u about 1 year ago

@kibi: can you please check that intrigeri's fix is ok?

#10 Updated by intrigeri about 1 year ago

  • Target version changed from Tails_3.10.1 to Tails_3.11

#11 Updated by CyrilBrulebois about 1 year ago

  • Assignee changed from CyrilBrulebois to intrigeri
  • QA Check changed from Ready for QA to Pass

Yeah, I can see how the referenced commit could have made things overflow. ACK for limiting the width the way you suggested.

(And thanks for your patience.)

#12 Updated by intrigeri about 1 year ago

  • % Done changed from 50 to 70

Merged into t-p-s master, released, uploaded to a dedicated APT overlay suite, started Jenkins build&test.

#13 Updated by intrigeri about 1 year ago

  • Status changed from In Progress to 11
  • % Done changed from 70 to 100

#14 Updated by intrigeri about 1 year ago

  • Assignee deleted (intrigeri)

#15 Updated by CyrilBrulebois 12 months ago

  • Status changed from 11 to Resolved

#16 Updated by intrigeri 11 months ago

  • Related to Bug #16311: Test suite: persistence.feature vs. buster, notifications… added

Also available in: Atom PDF