QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
534 stars 47 forks source link

Cannot change border size in qubes-desktop-linux-i3 #6589

Closed jmickelin closed 1 month ago

jmickelin commented 3 years ago

Qubes OS version R4.0

Affected component(s) or functionality qubes-desktop-linux-i3 version 1000:4.16-9.fc25.

Brief summary It seems the default_border setting and the border command are ignored.

How Reproducible Every time

To Reproduce Steps to reproduce the behavior:

  1. Add the following lines to ~/.config/i3/config in dom0:
    default_border pixel 10
    for_window [ class="Firefox" ] border pixel 30
  2. Open a terminal
  3. Open a Firefox window

Expected behavior The terminal window should have a border size of 10 pixels.

The Firefox window should have a border size of 30 pixels.

Actual behavior Border size is unaffected in both cases.

Additional context I think this is caused by the following line in the Qubes patch for i3: https://github.com/QubesOS/qubes-desktop-linux-i3/blob/93b8b1ba9d41f56c85bea4752a7270fe1468fb36/0001-Show-qubes-domain-in-configurable-colored-borders.patch#L180

Following the Git blame, it seems that this was first introduced when bumping the upstream version to 4.16. Comparing it with the patch for version 4.15 and using the then-upstream repo as reference, it looks like the previous behavior was to only override the width when the upstream one set it to a constant value, and using the user-supplied value in all other cases. The bump to 4.16 changed this to be overridden in all instances, presumably by mistake.

In order to follow a similar behavior as in previous versions, the patch should instead produce code akin to:

static int border_width_from_style(border_style_t border_style, long border_width, Con *con) {
    if (border_style != BS_NONE && border_width >= 0) {
        return logical_px(border_width);
    }

    const bool is_floating = con_inside_floating(con) != NULL;
    /* Load the configured defaults. */
    if (is_floating && border_style == config.default_floating_border) {
        return config.default_floating_border_width;
    } else if (!is_floating && border_style == config.default_border) {
        return config.default_border_width;
    } else {
        /* border_style is BS_NONE or the default styles are not applicable  */
        return 3;
    }
 }

(NOTE: The above code has not been tested.)

Solutions you've tried I configured both with really exaggerated border sizes (as above) to make sure I definitely would notice if it worked.

I have confirmed that the Firefox window is matched correctly by modifying other settings on it with for_window, it's just the border command that is broken.

I have successfully used these settings in i3 on non-Qubes machines previously.

Relevant documentation you've consulted https://i3wm.org/docs/userguide.html#_default_border_style_for_new_windows https://i3wm.org/docs/userguide.html#_changing_border_style https://i3wm.org/docs/userguide.html#for_window

Related, non-duplicate issues None

jmickelin commented 3 years ago

I'd be happy to supply a PR, but I'm not sure where to begin to set up a build environment for this. Let me know if you need the help and I can try reading up on it.

andrewdavidwong commented 3 years ago

I'd be happy to supply a PR, but I'm not sure where to begin to set up a build environment for this. Let me know if you need the help and I can try reading up on it.

I'm sure that would be appreciated, thank you! The dev documentation might have what you need to set up a build environment: https://www.qubes-os.org/doc/#developer-documentation

github-actions[bot] commented 1 year ago

This issue is being closed because:

If anyone believes that this issue should be reopened and reassigned to an active milestone, please leave a brief comment. (For example, if a bug still affects Qubes OS 4.1, then the comment "Affects 4.1" will suffice.)

jmickelin commented 10 months ago

Affects 4.1

Sorry, I never got around to figuring out how to set up a build environment for dom0, so I could never test the code and I sort of let this ticket stagnate. The upstream code is still missing from the Qubes-patched version, so it should be reincorporated following my previous sleuth work above. Though it might need to be massaged a bit in case it's bitrotted in the time since I posted.

neoniobium commented 9 months ago

Affects 4.2.