Problem/Motivation
1) On clean Drupal 7.12 install enable Content Translation module
2) Installed the second language (Latvian) (Default - English)
3) On admin/config/regional/language/configure enabled "Determine the language from the URL (Path prefix or domain)"
4) For the Basic Page content type - enabled Multilingual support -> Enabled, with translation
5) Create "Basic Page" node - and choose the language "English"
6) Save the node
7) Click on the node "Translate" tab
8) Click "add translation" link on opposit the non-default language (Latvian in my case). - The screen opens without the overlay!
When you click on the "edit" opposit the English (the default language) - it opens in the overlay!
Proposed resolution
Hope to get soon from the community ... :)
Comment | File | Size | Author |
---|---|---|---|
#33 | overlay-path-prefix-3.patch | 2.48 KB | c960657 |
#22 | overlay-path-prefix-2.patch | 2.39 KB | c960657 |
#18 | overlay-path-prefix-1.patch | 2.36 KB | c960657 |
#14 | add-translation-overlay.jpg | 270.68 KB | KrisBulman |
#14 | add-translation-clicked-no_overlay.jpg | 99.92 KB | KrisBulman |
Comments
Comment #1
xjmThanks @Nor4a for the clear report!
Comment #2
xjmHere's the similar issue that was resolved last spring: #759844: Overlay does not work with prefixed URL paths.
Comment #3
nod_For those watching at home, confirmed in d8 and the issue is indeed that urls with language prefix are not recognized by
Drupal.overlay.isAdminLink
event if after the prefix they are indeed admin links.It's either a tweak to the regxp, a monkey-patch or (from what I read in the other issue) rethinking the way urls are handled by drupal.
Comment #4
nod_wow, this sent me way way down the rabbit hole.
anyway, here is a fix, it's not pretty (only checking the end of the path, not the whole path) but hey it works for now. I changed stuff that was hurting my eye in the process. I know it won't go in like that but that's just for people to work with it, there is a bigger issue underneath anyway.
Comment #5
nod_#918738: Overlay does not allow language switching Same issue, different use-case.
Comment #6
xjmCool.
Comment #7
cosmicdreams CreditAttribution: cosmicdreams commentedI'm in the process of manually testing this one, thanks xjm for pointing me to it. I'll update this post with the result.
Before applying this patch: I can confirm this issue is happening (7.12)
Applied the patch...
Patch fixed the issue described in OP. Testing for regressions now...
Tested many of the typically troublesome pages (Dashboard, Reports > Recent log messages, Content) and I found no regressions.
Comment #8
cosmicdreams CreditAttribution: cosmicdreams commentedMarking as RTBC, this one's ready to commit IMO.
Comment #9
xjmExcellent; thanks @cosmicdreams!
Comment #10
catchThanks! Committed/pushed to 8.x, this should be an easy re-roll for 7.x so tagging as novice for the backport.
Comment #11
Purencool CreditAttribution: Purencool commentedComment #12
Purencool CreditAttribution: Purencool commentedThis patch has been tested in 7.x and was correct
Comment #13
Purencool CreditAttribution: Purencool commentedpatch above needs review
Comment #14
KrisBulman CreditAttribution: KrisBulman commentedI'm still experiencing the problem after the patch & clear cache in d7
$ git apply -v core-js-overlay-lang-prefix-1431076-4-version-2.patch
Checking patch modules/overlay/overlay-parent.js...
Applied patch modules/overlay/overlay-parent.js cleanly.
Can someone else please test and confirm
Comment #15
nod_Have you cleared you browser's cache too? the iframe gets in the way.
Comment #16
nod_Comment #17
KrisBulman CreditAttribution: KrisBulman commentedI get the same results, here..
my tests use the exact steps as listed in the original ticket..
$ drush cc all
* cleared browser caches
** tried this in these browsers: chrome, safari, firefox 10
Also added multiple other languages (ltr & rtl), and get the same effect (does not appear in overlay) when clicking 'add translation' from those either (after clicking translate on the node)
needs further testing by others to rule out a local development environment issue.
Comment #18
c960657 CreditAttribution: c960657 commentedNot quite. Doing a substring search only works when you are on the default language (where there is not path prefix) and the link is to a page that has a path prefix.
I wrapped up a patch that checks for enabled path prefixes. Now we no longer used Drupal.settings.pathPrefix that was added in order to support path prefixes in Overlay (see #759844: Overlay does not work with prefixed URL paths), but it seems like a useful addition, so I have left that in.
What was the reason for this change? It isn't mentioned above and seems unrelated to the path prefixing stuff.
Comment #20
c960657 CreditAttribution: c960657 commented#18: overlay-path-prefix-1.patch queued for re-testing.
Comment #22
c960657 CreditAttribution: c960657 commentedOnly check for prefixes when Locale module is enabled.
Comment #23
ryan.gibson CreditAttribution: ryan.gibson commented#22: overlay-path-prefix-2.patch queued for re-testing.
Comment #24
ryan.gibson CreditAttribution: ryan.gibson commentedI experienced the behavior mentioned without the patch.
After applying the patch to a clean Drupal install, the overlay displayed perfect on step 8. :)
Comment #25
xjm@ryanissamson: Excellent. Which browsers did you test in?
Comment #26
ryan.gibson CreditAttribution: ryan.gibson commented@xjm thanks for asking because I had forgotten - I originally tested in safari and chrome - the patch was successful.
I just tested in FF and on the final step, the error was present, the overlay disappeared.
Comment #27
xjmHm, did you make sure to clear the site cache after applying the patch? If so, let's have one more person confirm whether FF is broken (and mark the patch NW if so).
Comment #28
ryan.gibson CreditAttribution: ryan.gibson commentedYea, I thought the same thing, but after clearing site and browser cache it still didn't work. I am going to run the test one more time myself to be sure.
Comment #29
ryan.gibson CreditAttribution: ryan.gibson commentedOkay, so after retesting - I apparently made a mistake - FF is working fine now.
So, in Webkit and FF the patch is good; the overlay display perfect when reproducing the steps.
Comment #30
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedI can confirm the results of ryanissamson. Did the steps to reproduce with the patch applied on Drupal 8 in the following browsers:
Chromium 17 on Linux
Firefox 10 on Linux
Opera 11 on Linux
Internet Explorer 8
Internet Explorer 9
All tries opened in the overlay!
Comment #31
KrisBulman CreditAttribution: KrisBulman commentedapplied D8 patch & tested steps in FF10, Chrome, Safari 5.1.3 (OS X), IE8 (Windows): overlay stays active
This patch will need to be backported to d7
Comment #32
webchickExcellent! Thanks for all the through testing, folks!
Committed and pushed to 8.x. This needs a re-roll for 7.x.
Comment #33
c960657 CreditAttribution: c960657 commentedBackported to D7. The patch differs a bit, because locale_language_negotiation_url_prefixes() is D8-only (introduced in #1301460: Decouple domain/path language negotiation storage from language config storage), and the LANGUAGE_NEGOTIATION_URL_PREFIX constant was renamed for D8 (#1431040: Rename LOCALE_LANGUAGE_NEGOTIATION_* constants to LANGUAGE_NEGOTIATION_*).
BTW, I noticed that Drupal.settings.pathPrefix contains e.g. "de/" (with a trailing slash), but Drupal.settings.pathPrefixes (introduced in this ticket) contains e.g. ["da", "de"] (without trailing slashes). Both variants seem to be used in core. Also, they seem to be referred to with the less accurate url_prefixes. Do you think we should do anything about it?
Comment #34
c960657 CreditAttribution: c960657 commentedComment #35
KrisBulman CreditAttribution: KrisBulman commentedok, tested this patch in d7
patch applied cleanly, i had to fully clear the cache of all browsers i tested in order for it to work
browsers tested:
Chromium 17 on OSX
Firefox 10 on OSX
Safari 5.1.3 on OSX
Internet Explorer 8
Comment #36
webchickAwesome, thanks.
Ideally for D7 we'd have IE7 and IE6 testing, as well. However, this patch seems to be only moving JS code around, not changing it at all, so it shouldn't make Overlay support in those browsers any buggier than what they already are.
Committed and pushed to 7.x. Thanks! Another major bites the dust. :D
Comment #37
KrisBulman CreditAttribution: KrisBulman commentedOops! I guess I had D8 on the brain. Even though this is committed, I put it through the ringer on IE6 & IE7 as well & they passed the test!
Comment #38
webchickOh, perfect then! Thanks, Kris! :)
Comment #42
Tor Arne Thune CreditAttribution: Tor Arne Thune commented