#1979468: ".active" from linkGenerator(), l() and theme_links() forces an upper limit of per-page caching for all content containing links changed drupalSettings.currentPath to drupalSettings.path.currentPath, escapeAdmin.js was not updated.

CommentFileSizeAuthor
#7 core-js-fix-escapeAdmin-regression-rollback-2169299-4.patch467 bytesnod_
core-js-fix-escapeAdmin.patch1.39 KBnod_

Comments

jessebeach’s picture

jessebeach commented
Status: Needs review » Reviewed & tested by the community

Code looks good. Safe to commit went the bot comes back green.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Pffft. Sites. Who needs to go back to those anyway? ;)

Committed and pushed to 8.x. Thanks!

droplet’s picture

droplet commented
Status: Fixed » Needs work

The patched relled-back
https://drupal.org/comment/8349413#comment-8349413

nod_’s picture

nod_
Primary language French
Location Lille
commented

fix RTBC in #2169299: Fix "Cannot read property 'currentPathIsAdmin' of undefined" error in escapeAdmin.js.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Closed (fixed)

I'll close it, let's get the other one in.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Title: Back to site button broken » Revert: Back to site button broken
Status: Closed (fixed) » Active

According to #2169299-7: Fix "Cannot read property 'currentPathIsAdmin' of undefined" error in escapeAdmin.js we should revert this.

Regression cause by the roll back of a critical task, and pretty much the reverse of the critical bug #2167611: Back to site button broken follow-up of the critical task.

Back button is currently broken, this patch fixes it. I thought critical was fair, every time I broke the overlay that was critical soooo…
#1772466: Broke overlay with the dependency patch
#2004098: Overlay forms broken in IE10 for Windows 7
#1740958: overlay is broken

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Active » Reviewed & tested by the community
StatusFileSize
new core-js-fix-escapeAdmin-regression-rollback-2169299-4.patch467 bytes

Posting patch from other issue. The active class issue needs to be rerolled anyway so let's get this one in. It's ridiculous to have that button broken for so long.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Title: Revert: Back to site button broken » Back to site button broken
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Thanks for the patch @nod_

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Hm. I'm a bit confused how this happened, but committed and pushed to 8.x again.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Back to site button broken » Tests needed for Back to site button broken
Priority: Critical » Major
Status: Fixed » Needs work
Issue tags: +Needs tests

And actually, what about some tests that check for the existence of the back to site link when it ought to exist?

nod_’s picture

nod_
Primary language French
Location Lille
commented

You can't test it, it's all happening on the JS side.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Tests needed for Back to site button broken » Back to site button broken
Priority: Major » Critical
Status: Needs work » Fixed
Issue tags: -Needs tests

Dang. So not even an empty div or anything? That's unfortunate. :(

nod_’s picture

nod_
Primary language French
Location Lille
commented

There is all the HTML for the back button, but it's always there, the JS tells it to show or not when appropriate. And unless you have a JS-capable browser that can't be tested.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

We can fix the testing problem let's remove back button by adding overlay. :P

Thanks for the commit @webchick.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

dcrocks’s picture

dcrocks commented
Status: Closed (fixed) » Active

Did this get reverted? Because current code does not have this change.

dcrocks’s picture

dcrocks commented
Status: Active » Closed (fixed)

Sorry, #1979468: ".active" from linkGenerator(), l() and theme_links() forces an upper limit of per-page caching for all content containing links reverted this.

droplet’s picture

droplet commented
Related issues: +#2176721: 'Back to site' button does not preserve URL's language prefix and querystring