Incorrect behaviour for block page visibility

Created on 11 July 2014, almost 10 years ago
Updated 2 May 2024, about 2 months ago

Problem/Motivation

When leaving the "listed pages" text area empty in "Pages" block visibility, the options "Show for the listed pages" and "Hide for the listed pages" do the exact opposite of what they claim, they work as though there was an entry * in the list, i.e.:

  • Show for the listed pages will result in the block showing on all pages when the list is empty
  • Hide for the listed pages will result in the block showing nowhere when the list is empty

For an empty list, "Show on the listed pages" should not show the block anywhere. And, "Hide on the listed pages", should have the block show up on every page, when the list is empty.

Proposed resolution

Add a new radio button option for "Show on all pages", select this by default, hide the text area when this is selected, then validate that the box is not empty if either of the other two options are selected. See #19 πŸ› Incorrect behaviour for block page visibility Needs work

One consideration is that the default currently causes the block to show everywhere, which in itself is probably a good default. It's just that it is being achieved by combining "Show on the listed pages" with an empty list, which at least caused me to select "Hide on the listed pages", expecting that to result in the block showing everywhere.

Another consideration is to ensure that the descriptive test (Not Restricted/Restricted to certain pages) in the Pages tab is correct.

Remaining tasks

  1. code review
  2. add change record - suitable for a novice. There are instructions β†’ for adding a change record.

User interface changes

Before

After

API changes

None.

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
BlockΒ  β†’

Last updated about 6 hours ago

Created by

πŸ‡³πŸ‡±Netherlands eelkeblok Netherlands πŸ‡³πŸ‡±

Live updates comments and jobs are added and updated live.
  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

  • Needs reroll

    The patch will have to be re-rolled with new suggestions/changes described in the comments in the issue.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States mlncn Minneapolis, MN, USA

    This is not merely unintuitive/confusing, if this problem occurs, the UX is telling people they do not even need to look at this (as noted by geekygnr in #24 πŸ› Incorrect behaviour for block page visibility Needs work and alison in #26 πŸ› Incorrect behaviour for block page visibility Needs work ).

    1. Leave "Pages" blank.
    2. Have the "Show for the listed pages" and "Hide for the listed pages" radio buttons set to "Hide for the listed pages"
    3. The summary of Visibility will tell you Pages are "Not restricted" when in fact it is restricted so severely it will not show on any page.

    Given that fixing this ought to change the behavior in the edge cases of blank lists, we should make that change, and so will also need an update hook to do so.

    Even if we do not change stored configuration we should have an update that provides a link to the change record informing people the user interface behavior has changed (whether anything is updated or not). The update hook should further specifically alert people that the configuration they have may be leading to an unexpected result of hiding their block from every page, if they have 'negate' ('hide_on_pages' => 'Hide on specific pages') checked and the Pages textarea blank.

    If we don't want to allow such a confusing case (the default of displaying everywhere is fixed by the 'Show on all pages' radio button in the patch) then we need a fourth radio button 'Hide on all pages' or we need to update such blocks to be disabled. (The latter is better in my opinion, provided we let people know which blocks were in such a state. But i'm not sure how that would interact with other visibility display settings, let alone more complex modules like Block Visibility Groups.)

    Where does the logic to invert the standard behavior for a blank list actually happen? I know it's sort of common in Drupal but whenever it is also paired with 'Negate the condition' the double negative (negate an empty list that is interpreted as everywhere == hide everywhere) is bound to be counterintuitive anywhere it comes up. I cannot find it in core/modules/system/src/Plugin/Condition/RequestPath.php or core/lib/Drupal/Core/Condition/ConditionPluginBase.php (which provides negate as a default option) nor in core/lib/Drupal/Core/Path/PathMatcher.php.

    Block visibility based on Vocabulary (using core/lib/Drupal/Core/Entity/Plugin/Condition/EntityBundle.php i think) has the same odd behavior (you can select nothing at all, choose negate the condition, and then the block is hidden everywhere) but at least it doesn't set any expectations at allβ€” no help text at all and no summary in the vertical tabs!

    Some or all of my comments are probably for follow-up issues. A lot of work has been put into this, especially by paulocs; it would be wonderful to get this across the finish line.

  • πŸ‡ΊπŸ‡ΈUnited States alison

    MR has conflicts even with 9.x (but also needs to be redone against 11.x) -- anyway, adding "needs reroll" tag (right?)

  • Pipeline finished with Failed
    5 months ago
    Total: 184s
    #89085
  • πŸ‡¨πŸ‡¦Canada geekygnr Waterloo

    I don't think I did it particularly well but I have started a re-roll to 11.x

    Testing the UI I think it is working as expected but there is probably still a lot of fixing to do.

    I cherry picked the commit but the merge conflicts were difficult to resolve. It will need additional work.

  • Pipeline finished with Failed
    5 months ago
    Total: 203s
    #89808
  • Pipeline finished with Failed
    5 months ago
    #89823
  • Pipeline finished with Failed
    about 2 months ago
    Total: 593s
    #161122
  • Pipeline finished with Failed
    about 2 months ago
    Total: 665s
    #162042
  • πŸ‡¨πŸ‡¦Canada geekygnr Waterloo

    I rolled out the 11.x fix and got all the tests to pass.

Production build 0.69.0 2024