Problem/Motivation

From #735528-148: FAPI #states: Fix conditionals to allow OR and XOR constructions by neochief:

I've just caught another flaw, which might affect group conditions due to unproper usage of variables context. The scope of "selector" and "case" variables in function states.Dependent.prototype.initializeDependee() currently is Closure, which causes bad things when you process group conditions in that function, since Closure scope gets variables from last group.

nod_ also confirmed the bug.

Proposed resolution

  • Attached patch (rerolled against D8) should resolve the issue.

Remaining tasks

  • Patch needs code review.
  • Patch need manual testing to ensure there are no regressions.
CommentFileSizeAuthor
#11 1542676-11.patch681 bytesrpayanm
#9 states-scope-followup.patch1017 bytesboombatower
states-scope-followup.patch701 bytesxjm

Comments

xjm’s picture

xjm
she/her
Primary language English
commented
Title: FAPI #states » FAPI #states scope issue in Dependent prototype

Didn't finish typing a title. Someone who understands JavaScript is welcome to retitle to better reflect the issue. ;)

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs manual testing
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

no surprises here, fixing a bug or not this is the right way to code it.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Sorry this could really use manual testing to confirm no regressions, happy to commit once that's done.

neochief’s picture

neochief commented

Well, this one works on my site just fine.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

testing in #5

The patch would make #states fail completely on any browser if there was an issue with it. Since it's working, it's all good.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

Moving back to 7.x.

boombatower’s picture

boombatower commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new states-scope-followup.patch1017 bytes
mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

No longer applies.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 1542676-11.patch681 bytes
mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue tags: -Needs reroll

  • Dries committed d67ed98 on 8.3.x 
    - Patch #1542676 by xjm: Fixed FAPI #states scope issue in Dependent...

  • Dries committed d67ed98 on 8.3.x 
    - Patch #1542676 by xjm: Fixed FAPI #states scope issue in Dependent...

  • Dries committed d67ed98 on 8.4.x 
    - Patch #1542676 by xjm: Fixed FAPI #states scope issue in Dependent...

  • Dries committed d67ed98 on 8.4.x 
    - Patch #1542676 by xjm: Fixed FAPI #states scope issue in Dependent...

  • Dries committed d67ed98 on 9.1.x 
    - Patch #1542676 by xjm: Fixed FAPI #states scope issue in Dependent...

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.