Fix the report page when authorize.php completes an update manager operation

subscribing

Possibly related: #679190: Overlay breaks the Update manager workflow

following

following, hoping to help out where I can.

Any update on this one? I'm not really sure why it's a release blocker that the page is ugly, and there doesn't seem to be any suggestions here to improve it.

Subscribination

Downgrading. Adding bling is not critical.

Design is not bling.

It is not bling, that's true, but it is also not going to hold us back from releasing Drupal 7. Let's please not play anymore status ping-pong.

Can we focus on some sort of a way forward here? I can't really even parse a meaningful bug report from this issue, apart from HTML compliance.

Here is a first step towards getting it looking acceptable. Note that #606190: Fix handling of database schema updates in update manager workflow is still open, so these patches ignore the specific tasks listed on the page, and focuses on just getting the markup acceptable.

Can you post a screenshot of what the patch is meant to do please?

Here are some of the problems I think we should try to address about this page:

Install/enable: what do we actually mean by these terms? Does 'install' mean *just* download and put the folder in the right place? I'm sure I've seen lots of documentation that says 'install Views' to mean 'download, put in place, enable'. We should be careful what we mean here.

- How about changing the title to "Drupal Update Manager"?
- I suppose instead of having a title for each module, we could put them all under a New Modules title?
- I think the repetition part isn't actually repetition. The first is a status message showing you everything worked out, the second instance is an itemized list of what was installed.
- Install/Enable are used correctly here, IMHO.
- Admin/Front page links are under debate at #606190: Fix handling of database schema updates in update manager workflow

Attached is a screenshot of what #14 is intended to do, plus my suggestions for the problems joachim raised as listed above.

Also, anyone know why the test failed on #14?

EDIT: I think the "error log below" is actually what I have listed as "New Modules", so I think changing it to "Update Manager Log" would be a much better choice.

Attached patch does the following:
- Same as #14
- Plus page title added in update manager so the default in authorize.php isn't used.

After second thought the title for each module is there because that's a generic theme function in includes/theme.maintenance.inc used for all authorize.php reports, and so I didn't change because I didn't want to lock down that theme function here.

Woah, test fail is gone. Rogue test bot maybe? At any rate, since its now a lovely shade of green, switching back to needs review.

This patch implements #18 and #21

Strange, I get an undefined function error for check_markup() in #22.

Yeah I thought that was strange too, but just followed the precedent set with "$output .= '<h3>' . check_plain($heading) . '</h3>';".

At any rate, this patch fixes:
- invalid html pointed out in #3
- unfriendly title from #16
- theme function weirdness from #21
- unneeded security functions from #24

Awesome work, rschwab!

Committed to HEAD. Thanks!

Wait what!? Ugh, why are we rushing in string changes this last minute. Can anyone tell me about the following conciderations:

1) Is this done consistently over all pages?
2) Are users in actually in an "Update" mode or are they in a "Install" mode. Now we just repeat the module name?
3) Do we ever captialize the 2nd, and 3rd word?

Sorry I am just trying to keep up a quality standard, especially this late in the process we should rush semi-improvements in that will still cause confusion.

Alright, moving to Drupal 8.

I fully understand the reasoning, I hope you understand mine - we are around on IRC a lot so just grab us when needed (asking us shouldn't necessary mean perfection, rather answering the question: are we really solving the user problem?). So lets mark this need work for D8, because now we moved it from scary to confusing - which is mostly just moving the problem, but agreed in the right direction :)

We might want to do a small quick follow up on the capitalization.

dww, Bojhan, et al: Do you have any suggestions for improving the titles?

for installs its currently "Drupal Update Manager"
for updates its currently "Authorize file system changes"

'Update manager' (we have a guideline to not use the word 'Drupal', it is mostly redundant and complicates other-named install profiles)

'Allow changes to files' ?

'Update manager' could work for both I suppose. 'Allow changes to files' doesn't really make sense to me, because the file changes have already been made by the time the user reaches this screen.

And just in case everyone likes that - here is a patch to change titles to "Update manager".

I'd be fine with that. Dww explained in IRC that 'Authorize file changes' was a bug anyway - should ideally not be seen. Yes, I think 'Update manager' works for both.

I think this is a good string change. "Drupal Update Manager" would have been weird.

I would RTBC, but want Bojhan to sign off that this is all that is needed.

1 consistent, simple page title throughout the process instead of scary talk. This is nice and ready again.

Committed to HEAD. Thanks!