During the install phase (install.core.inc), help references to INSTALL.txt use a link – <a href="@install_txt">INSTALL.txt</a> – while the status report in system.install does not. This patch makes system.install consistent with the installer. It's helpful from a usability standpoint since INSTALL.txt doesn't exist in the root directory, and the link takes you directly to the file.

CommentFileSizeAuthor
#1 install-txt-link-1981912-1.patch1.49 KBkostajh

Comments

kostajh’s picture

kostajh commented
Status: Active » Needs review
StatusFileSize
new install-txt-link-1981912-1.patch1.49 KB
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Component: system.module » documentation

Moving to documentation component, since it seems more related to that than system.module.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue tags: +Needs manual testing

Can someone manually test and verify that this link works -- if you click it, does the INSTALL.txt file appear in your browser window? If so, it seems like a good idea.

kostajh’s picture

kostajh commented

For what it's worth, I tested this manually.

For someone else who would like to:

  1. Run chmod go-w /path/to/drupal8/sites/default
  2. Run chown -R root /path/to/drupal8/sites/default and make sure that sites/default/settings.php doesn't exist.
  3. Start the install process. On Verify Requirements, you'll see a link to INSTALL.txt
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue tags: +Novice

I'm glad you have tested it! Since this involves a code and not just documentation change, I am not going to commit it until someone else other than the person who made the patch has done the same test (including clicking the link to verify the INSTALL.txt file is shown) and marked this issue "reviewed and tested by the community".

I'm marking this "Novice" in order to (hopefully) attract someone who can do this review, which should be fairly simple to do.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: documentation » system.module

And moving it back into system.module in case that helps people know "it really does need a review". :)

themusician’s picture

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

I tried this on a vanilla 8.x install built from the most recent pull and it worked just fine. The install.txt file was viewable via a link on the verify requirements page.

Anonymous’s picture

Anonymous (not verified) commented

I tried this on a vanilla 8.x install - the link displayed on the Verify Requirements page and successfully displayed the install.txt file when accessed.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: system.module » documentation
Assigned: Unassigned » jhodgdon
Issue tags: -Needs manual testing

Thanks to both of you for testing! I'll get it committed shortly unless one of the other core committers wants to do it.

themusician’s picture

themusician commented

Glad to help. At our local meetup I was running a ladder session on reviewing issues and testing patches so it is fun to see a quick turnaround on an issue.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 8.x. As this is a UI change and not critical or a bug/error/typo, we will not be backporting it to 7.x.

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