Remove Overlay

I'm staying out of this one, but in fairness, I also pointed out that Overlay is quite handy from contextual links, and that most of the rage is because it opens all the time everywhere. So I'm +1 to the "use only for contextual operations." And I believe I said the chrome "could" be no longer needed, not "is." :)

+1 and I want to RTBC it.

I was really into the idea of using a stripped down overlay for just the contextual links. #1889150: Simplify overlay look, *only use for contextual operations*

I'd support removing overlay, disabling it is usually the first thing I do on a site.

Funny thing is the Spark team at Acquia worked really hard to effectively remove the overlay in Drupal 8 (especially @tkoleary and myself). The problem with removing the overlay wholesale is we still think contextual operations make sense (and they should let you quickly get back to your page). So if you hit the "Configure block" contextual link on a block, a *modal dialog* should open and let you do your stuff, submit and close. If the contextual link leads you to a whole new page, that is not very useful. So why not use the Drupal 8 modal for these thing?. We reiterated this *several times* in our meetings and came to these two key points:

- these modals need to show up on a frontend themed Drupal website; we don't have a 100% surefire way to ensure we can display a modal that is themed like a backend UI and does not interfere with the underlying page or vice versa; and Drupal can swap your backend looks independent of your frontend looks, so these are roles of two different themes; an iframe serves this purpose of sandboxing in HTML basically
- the CSS technologies available to do responsive pages does not let us work with "width of a part of the page", it only let us work with width of the whole page; so if the backend modal content needs to adapt to the available width of the modal, it can only do so without JS fiddling if the modal is an iframe

Then it would theoretically still be possible to put an iframe into a modal and ignore the overlay, but then the whole content of the modal would be an iframe, so the modal would have very tiny use in the whole process and most of its backend would need to be ignored and a whole iframe based thing implemented. Which is .... already implemented in overlay.

So therefore the plan in #1889150: Simplify overlay look, *only use for contextual operations* was to make the overlay totally look like a modal and only use it when a modal needs to happen on top of the frontend. It would not be the overlay as we know it in Drupal 7 (and currently in 8). @tkoleary posted this great piece at https://groups.drupal.org/node/294848 explaining all the things.

The funny thing is Spark has been really putting in lots of time and resources to make the overlay go away :) There is maybe not much demonstration of the community opposition to that idea in the groups post, but if you look at Ryan Weal's related issue to not include the overlay in the standard profile at #2004836: Disable overlay in standard profile you'll see how much community opposition there is.

In the end this was de-prioritized from the Spark queue to avoid fighting with community members who wanted to keep the overlay. Such irony.

YAY, +1 for removing the overlay. I've always been against it since I started using Drupal! :-)

If it can't be removed, then its usage should be restricted to contextual operations. Maybe (but this is only a random thought that should be verified) restricting its usage would make it easier to use for screen reader users! :-)

And, if its usage can't be restricted to contextual operations, then it should be disabled in the standard profile. Not to mention that using Voiceover + OS X + Safari it can *easily* make the browser crash! Not sure if this depends on my local environment, though.

i wish that dream would ever come true:P
+10000
contextual links or not..just ?destination works for me there.

related #2004836: Disable overlay in standard profile

Major +1 from me, too. If overlay wants to exist, it should exist in contrib. It's much too fragile to be part of core.

To add to the list of things overlay makes harder:

• searching within the overlay using the browser's search is inconsistent, and occurrences of the search in the page behind the overlay are also found.
• refreshing pages where an overlay is displayed causes two Drupal page loads—one for the page behind the overlay, one for the overlay. This can sometimes cause disastrous results, if the page behind does something strange.
• scrolling can behave strange when inside the overlay.

I'll +1 the basic idea as well here.

I've found the the overlay mostly just interferes with the development process, can have inconsistent behavior at times, and consequently is usually the first thing I shut off when starting a new client site.

The only time I've ever intentionally enabled overlay was for a client who really liked the 'shiny stuff', and they were impressed by overlay.

I'm all for downgrading it to contrib so the people that want it can have it, while not adding a burden to the core development team.

Every feature is a burden on the "core development team".

The main benefit of the overlay is for new users, new users don't even know what a contrib module is. Moving it to contrib just because it's annoying for experienced devs to turn it off is backwards logic.

While every feature requires maintenance, there are plenty of features in core that core developers use regularly - whether it's the comment form, Seven, or the Views UI.

There's a massive difference between maintaining something you use, vs. maintaining something you're not bothered about, vs. maintaining something you go out of your way to avoid/disable.

There's a massive difference between maintaining something you use, vs. maintaining something you're not bothered about, vs. maintaining something you go out of your way to avoid/disable.

I think this is where we disagree. Core development is not just for core developers, we build a framework for a lot of users who do not develop for core.

I'm not saying it's not a valid factor, maintenance is always an issue, but it should not be the only factor in a decision.

Core development is not just for core developers, we build a framework for a lot of users who do not develop for core.

Yes we do. And that's why usability testing is great - to ensure that core UIs don't cater for core developers. Again that's completely different from adding/keeping UIs that core developers actively avoid using because they get in the way.

We talked about this on IRC, and I believe we agreed to do this.

Postponing on #1889150: Simplify overlay look, *only use for contextual operations* since that eliminates 99.9% of the complaints with Overlay.

If that doesn't get done by release, then we'll revisit this one.

Would be good to still have a passing .patch, hopefully... http://github.com/RobLoach/drupalmirror/compare/2088121

Two changes:

1. Drupal\system\Tests\Upgrade\FilledStandardUpgradePathTest
2. Remove overlay references elsewhere

Didn't mean to change status.

Meant to upload the .patch, not the .diff.

For testbot

Back to postponed

Does work on this and #787896: Add a link so that administrators can return to their most recently visited non-admin page mean that #1889150: Simplify overlay look, *only use for contextual operations* should be closed (wont fix)?

edit: fixed issue #

I don't think everything is gone yet. Using ack to search for overlay found a few things.

core/modules/system/system.api.php
2967:  // If the current page request is inside the overlay, add ?render=overlay to
2968:  // the success callback URL, so that it appears correctly within the overlay.
2969:  if (overlay_get_mode() == 'child') {
2971:      $batch['url_options']['query']['render'] = 'overlay';
2974:      $batch['url_options']['query'] = array('render' => 'overlay');

core/modules/system/tests/upgrade/drupal-7.language.database.php
191:  'location' => 'modules/overlay/overlay-parent.js',
223:  'location' => 'modules/overlay/overlay-child.js',
231:  'location' => 'modules/overlay/overlay-child.js',
387:  'value' => 'a:16:{i:0;s:14:"misc/drupal.js";i:1;s:14:"misc/jquery.js";i:2;s:19:"misc/jquery.once.js";s:10:"refresh:ca";s:7:"waiting";i:3;s:29:"misc/ui/jquery.ui.core.min.js";i:4;s:21:"misc/jquery.ba-bbq.js";i:5;s:33:"modules/overlay/overlay-parent.js";i:6;s:32:"modules/contextual/contextual.js";i:7;s:21:"misc/jquery.cookie.js";i:8;s:26:"modules/toolbar/toolbar.js";i:9;s:32:"modules/overlay/overlay-child.js";i:10;s:19:"misc/tableheader.js";i:11;s:17:"misc/tabledrag.js";i:12;s:12:"misc/form.js";i:13;s:16:"misc/collapse.js";s:10:"refresh:cv";s:7:"waiting";}',

Also I am seeing (via manual testing) that tour module is now throwing this error on every page.

Uncaught DrupalBehaviorError: attach ; tour: Cannot call method 'querystring' of undefined

I cant reproduce it either

+++ b/core/modules/tour/tests/tour_test/tour_test.info.yml
@@ -4,6 +4,6 @@ description: Tests module for tour module.
-hidden: TRUE
+#hidden: TRUE

lets restore that

Besides that looks good

also we should probably remove this hook_alter implementation from toolbar:

/**
 * Implements hook_system_info_alter().
 *
 * Indicate that the 'page_top' region (in which the toolbar will be displayed)
 * is an overlay supplemental region that should be refreshed whenever its
 * content is updated.
 *
 * This information is provided for any module that might need to use it.
 */
function toolbar_system_info_alter(&$info, $file, $type) {
  if ($type == 'theme') {
    $info['overlay_supplemental_regions'][] = 'page_top';
  }
}

Thanks!
Lets do it.

This feels so awesome..

We can also commit this now and mark the issue you linked as critical, to make sure D8 wont ship before it gets committed ;)

I don't think this needs Dries yet. And yes, the other patch should be committed first.

Back to Dries, and I guess postponed?

Sorry, due to #2140447: Open redirect in overlay (which was a critical security issue in D7, thus taking precedence over this patch) this will need a re-roll. :(

Very excited about this issue... re-rolled!

Apparently I do not have the correct rights to mark it "Postponed". :D

nod_ suggested that we might now have to postpone twice, for it to really work.

Committed to 8.x! :)

This issue needs to be added to https://drupal.org/node/2116417 but unfortunately we can't do that until #2135759: DATA LOSS: Change record issue reference field is single-value is resolved.

So, I was thinking overlay was moved to contrib. but I just want I verify that is *not* the case.

Overlay is not gong to be a contrib module.(?)

I'd be interested in having the Overlay in contrib with the changes in #1889150: Simplify overlay look, *only use for contextual operations*

The reason I like the Overlay is because it uses an iframe. Any admin UI in the ctools modal gets completely destroyed by the front-facing theme.

I'm going to attempt to port Modalframe to Drupal 8 when I find the time. Maybe that could be the answer for a contrib-based Overlay-like thing?