Problem/Motivation

file fields prints out a seperate image file as an icon

<span class="file">
<img class="file-icon" alt="" title="text/plain" src="/core/modules/file/icons/text-plain.png"> 
<a href="http://drupal8.dev/sites/default/files/files/interdiff-field-117-121_0.txt" type="text/plain; length=3264">interdiff-field-117-121.txt</a></span>

No reason for that exatra http request everytime when it can be done in css & will be easier to modify for the themer

Proposed resolution

<a href="http://drupal8.dev/sites/default/files/files/interdiff-field-117-121_0.txt" type="text/plain; length=3264" class="file-icon-text">interdiff-field-117-121.txt</a>

css:

file-icon-text{
  background: url(icont-text-something.gif) no-repeat;
}

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#87 Screen Shot 2014-11-07 at 15.37.45.jpg445.14 KBLewisNyman
#86 use_css_for_file_icons-2236855_86.patch10.78 KBstefank
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,077 pass(es). View
#74 use_css_for_file_icons-2236855_74.patch10.75 KBstefank
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855_74.patch. Unable to apply patch. See the log in the details link for more information. View
#72 use_css_for_file_icons-2236855_72.patch12.88 KBstefank
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,374 pass(es). View
#68 2236855-68-before.png1.21 MBIeva Uzule
#68 2236855-68-after.png1.36 MBIeva Uzule
#67 interdiff_2236855_58_67.txt1.84 KBrachel_norfolk
#67 use_css_for_file_icons-2236855_67.patch10.73 KBrachel_norfolk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855_67.patch. Unable to apply patch. See the log in the details link for more information. View
#58 use_css_for_file_icons-2236855-58.patch8.89 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,014 pass(es). View
#51 use_css_for_file_icons-2236855-51.patch10.72 KBLewisNyman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-51.patch. Unable to apply patch. See the log in the details link for more information. View
#49 use_css_for_file_icons-2236855-49.patch2.13 KBLewisNyman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-49.patch. Unable to apply patch. See the log in the details link for more information. View
#49 interdiff.txt2.13 KBLewisNyman
#48 use_css_for_file_icons-2236855-48.patch10.8 KBrachel_norfolk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,618 pass(es). View
#48 interdiff-2236855-45-48.txt3.12 KBrachel_norfolk
#48 2014-09-20 at 15.18.png220.04 KBrachel_norfolk
#47 2014-09-20 at 12.00.png187.21 KBrachel_norfolk
#46 use_css_for_file_icons-2236855-45.patch10.09 KBrachel_norfolk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-45.patch. Unable to apply patch. See the log in the details link for more information. View
#46 interdiff-2236855-44-45.txt1.74 KBrachel_norfolk
#44 use_css_for_file_icons-2236855-44.patch8.36 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,929 pass(es). View
#41 2236855.37.patch14.56 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2236855.37.patch. Unable to apply patch. See the log in the details link for more information. View
#41 35-37-interdiff.txt4.25 KBalexpott
#36 files-icons-rtl.png159.96 KBrteijeiro
#35 css-file-icons-2236855-35.patch9.97 KBrachel_norfolk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,868 pass(es). View
#34 files-icon-rtl.png21.92 KBrteijeiro
#34 css-file-icons-2236855-34.patch9.92 KBrteijeiro
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,608 pass(es). View
#30 files-icons.png158.02 KBrteijeiro
#29 css-file-icons-2236855-29.patch9.84 KBrachel_norfolk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,606 pass(es). View
#29 css-file-icons-2236855-29-example.png491.95 KBrachel_norfolk
#27 patchcss-file-icons-2236855-27.patch7.54 KByuki77
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,565 pass(es). View
#22 css-file-icons-2236855-22.patch9.76 KBngocketit
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,850 pass(es). View
#15 css-file-icons-2236855-15.patch8.5 KBngocketit
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,741 pass(es), 2 fail(s), and 0 exception(s). View
#10 css-file-icons-2236855-10.patch6.95 KBngocketit
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,503 pass(es), 2 fail(s), and 0 exception(s). View
#6 css-file-icons-2236855-6.patch5.58 KBlauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch css-file-icons-2236855-6.patch. Unable to apply patch. See the log in the details link for more information. View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

cs_shadow’s picture

Seems fine to me. The only thing that's not clear to me is that how to tell CSS what path to use for the image. I'll be happy to provide a patch if the above point is clarified.

LewisNyman’s picture

@cs_shadow We tend to just use the relative path technique: ../../../misc/etc

cs_shadow’s picture

@LewisNyman I was not referring to how to specify the path of the image. I actually wanted to ask that how do we determine which icon to use which depends on file type. If we intend to do it this way we'll have to define as many css classes as many filet types we have. Also this will be difficult to extend when we want to add a few more file types IMO. Since css class will depend on the file type, whats the best way to accomplish this?

LewisNyman’s picture

Issue tags: +CSS, +frontend

@cs_shadow Looks like I never got back to you, sorry. I think it's ok to allow it to be extendable with new file types, as long as it doesn't look broken. We should use a default icon for all file types and then overwrite them for the ones we know about.

lauriii’s picture

Assigned: Unassigned » lauriii
lauriii’s picture

Assigned: lauriii » Unassigned
Status: Active » Needs work
FileSize
5.58 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch css-file-icons-2236855-6.patch. Unable to apply patch. See the log in the details link for more information. View

In my patch we add grouped class of the mime type for the link, and if there's no group, then we add a converted mime type class.

1. I'm not sure if we should have always a specific mime type class? The reason why we shouldn't add it always is that we have some mime types that are terribly long. Adding the specific classes could be a good idea in terms of adding extending the functionality, but in core we wouldn't need them. Any opinions?

2. Should we have always a file class included in the classes?

If there's any CSS wizards around, we could start implementing the CSS part.

JamesLefrère’s picture

Assigned: Unassigned » JamesLefrère
bdevore’s picture

Adding the classes would certainly make swapping them out easier if I didn't like the default implementation. Any thought about using font based icons rather than images?

ngocketit’s picture

This needs to be rerolled due to this: file.module - Convert theme_ functions to Twig

ngocketit’s picture

FileSize
6.95 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,503 pass(es), 2 fail(s), and 0 exception(s). View

New patch attached.

ngocketit’s picture

Assigned: JamesLefrère » Unassigned
Status: Needs work » Needs review

The last submitted patch, 6: css-file-icons-2236855-6.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: css-file-icons-2236855-10.patch, failed testing.

swentel’s picture

+++ b/core/modules/file/src/Plugin/Field/FieldFormatter/GenericFileFormatter.php
@@ -28,6 +28,10 @@ class GenericFileFormatter extends FileFormatterBase {
+    ); ¶

redundant space

ngocketit’s picture

FileSize
8.5 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,741 pass(es), 2 fail(s), and 0 exception(s). View

Thanks @swentel for noticing the typo. I removed it & updated the patch.

ngocketit’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: css-file-icons-2236855-15.patch, failed testing.

ngocketit’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: css-file-icons-2236855-15.patch, failed testing.

lauriii’s picture

  1. --- /dev/null
    +++ b/core/modules/file/css/file.generic_file.css
    

    This should be changed to something more descriptive such as file.icons.css

  2. +++ b/core/modules/file/src/Plugin/Field/FieldFormatter/GenericFileFormatter.php
    @@ -28,6 +28,10 @@ class GenericFileFormatter extends FileFormatterBase {
    +    $elements['#attached'] = array(
    +      'library' => array('file/drupal.file.formatter.generic_file'),
    +    );
    

    Tests are failing because of this

ngocketit’s picture

Assigned: Unassigned » ngocketit

Indeed! That makes the field shown even when there is no file attached and hence fails the test. Thanks @lauriii for that! I'll provide a new patch soon.

ngocketit’s picture

FileSize
9.76 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,850 pass(es). View

New patch added. The css file is renamed to file.formatter.generic.css since it's used for generic_file formatter.

ngocketit’s picture

Assigned: ngocketit » Unassigned
Status: Needs work » Needs review
lauriii’s picture

Issue tags: +Needs reroll
lauriii’s picture

Issue tags: +FUDK
yuki77’s picture

Assigned: Unassigned » yuki77
yuki77’s picture

Assigned: yuki77 » Unassigned
FileSize
7.54 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,565 pass(es). View

I hope it's working....

rachel_norfolk’s picture

Assigned: Unassigned » rachel_norfolk
Status: Needs review » Needs work

I think there are a couple of things missing in the re-rolled patch. On a site, I'm not seeing an image next to a file link, as we should expect.

I'm trying to include the extra bits now...

rachel_norfolk’s picture

Assigned: rachel_norfolk » Unassigned
Status: Needs work » Needs review
FileSize
491.95 KB
9.84 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,606 pass(es). View

okey dokey - this builds on yuki77's work and now includes a few more of the changes proposed in #1 but up to date with 8.0.x. I think so, anyway - it does show the file icons via css - as seen in attached png.

screenshot of patch working correctly

rteijeiro’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
158.02 KB

It looks good. Patch applies cleanly and code looks good.

lauriii’s picture

+1 for "I love Dries"

herom’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs reroll +Needs screenshots
+++ b/core/modules/file/css/file.formatter.generic.css
@@ -0,0 +1,57 @@
+  padding-left: 20px;
...
+  background-position: left center;

It feels like this might need RTL-specific CSS too. Can you post an RTL screenshot for this?

rachel_norfolk’s picture

Assigned: Unassigned » rachel_norfolk
rteijeiro’s picture

Assigned: rachel_norfolk » Unassigned
Status: Needs work » Needs review
FileSize
9.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,608 pass(es). View
21.92 KB

Now it's fixed.

rachel_norfolk’s picture

FileSize
9.97 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,868 pass(es). View

Ah - we need to remember to 'cancel' the left margin in [dir="rtl"] as well as set a right margin. Otherwise, we are introducing little extra blank margins after the link in rtl templates.

Come to think of it - do we need to think about this anywhere else?

rteijeiro’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
159.96 KB

Great! Thanks rachel_norfolk for fixing the issue correctly ;)

Now it looks great without extra paddings. It's a RTBC for me.

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Overall this seems like a great change, and thanks for all of the RTL testing as well.

A couple of questions though:

  1. +++ b/core/modules/file/file.module
    @@ -1589,80 +1578,50 @@ function template_preprocess_file_link(&$variables) {
    - * @param \Drupal\file\FileInterface $file
    + * @param \Drupal\file\File $file
    ...
    -function file_icon_path(FileInterface $file, $icon_directory = NULL) {
    ...
    +function file_icon_classes(File $file) {
    

    Is there a reason we removed the FileInterface type hint here in favour of File? That's unusual.

  2. +++ b/core/modules/file/file.module
    @@ -1693,7 +1652,7 @@ function file_icon_map(FileInterface $file) {
    -      return 'x-office-document';
    +      return 'office-document';
    

    Curious, why this change (and the other x-office -> office changes)? Looking at the search results for "x-office-document" on Google, this seems to be a pretty standard way to label these icons.

LewisNyman’s picture

Status: Needs review » Needs work

Sorry I didn't get a chance to review this, I found an improvement we can make to the CSS.

+++ b/core/modules/file/css/file.formatter.generic.css
@@ -0,0 +1,64 @@
+.file.package-x-generic {
+  background-image: url(../icons/package-x-generic.png);
+}

We should be using SMACSS variants for this instead of chaining classes, so: .file--package-x-generic

Good work! This will be a great improvement.

joelpittet’s picture

+++ b/core/modules/file/file.module
@@ -1589,80 +1578,50 @@ function template_preprocess_file_link(&$variables) {
+  // Set file classes to the options array.
+  if (isset($variables['attributes']['class']) && is_array($variables['attributes']['class'])) {
...
   }
-  return FALSE;
+  else {
+    $variables['attributes']['class'] = file_icon_classes($file);
+  }

If this was already an attribute object as I'm attempting to do over at #2108771: Remove special cased title_attributes and content_attributes for Attribute creation this could become:


$variables['attributes']->addClass(file_icon_classes($file));

But simply now you could clean it up with:

$variables['attributes'] = new Attribute($variables['attributes']);
$variables['attributes']->addClass(file_icon_classes($file));

Because we have an array() in the hook_theme().

And ditto on questions form @webchick in #37

joelpittet’s picture

Title: use css for file icons in file fields » Use CSS for file icons in file fields
Issue tags: -Needs screenshots
alexpott’s picture

Status: Needs work » Needs review
Issue tags: +Needs screenshots, +Needs change record
FileSize
4.25 KB
14.56 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2236855.37.patch. Unable to apply patch. See the log in the details link for more information. View

We can use the Attribute object to simplify everything - also we're calling $file->getMimeType a lot and in a loop during the preprocess I think we can remove this. Plus the logic is now a bit different - I think we should always add the full mime type to the class since this best reproduces the logic of testing whether or not a specific mime type icon file exists. If we keep the logic as in #35 then you won't be able to provide specific overrides only using css. #35 would also add the class returned by file_icon_map() and the category discovered in file_icon_class() - which seems wrong too (n.b. this could only occur for text files).

Patch attached addresses these issues and removes the now unnecessary array containing human-readable names, for use as text-alternatives to icons.

Also we need a change record because we removing a theme function and completely changing how file icons work.

alexpott’s picture

Issue tags: -Needs screenshots

The patch in #41 does not address the feedback from #37, #38, or #39.

Status: Needs review » Needs work

The last submitted patch, 41: 2236855.37.patch, failed testing.

lauriii’s picture

Status: Needs work » Needs review
FileSize
8.36 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,929 pass(es). View

Fixed the patch from #41 and changed CSS classes as suggested in #38. #39 was fixed already in @alexpotts patch.

LewisNyman’s picture

Status: Needs review » Needs work

Looks like the CSS file is missing from the patch

rachel_norfolk’s picture

Assigned: Unassigned » rachel_norfolk
FileSize
1.74 KB
10.09 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-45.patch. Unable to apply patch. See the log in the details link for more information. View

oh - the file.formatter.generic.css file seemed to go missing at some point.

I've done a bit of digging back and picked it up from #35. This means that the css changes do NOT include Lewis' SMACSS thing - I'm still working on that but wanted to drop this here in the meantime...

rachel_norfolk’s picture

FileSize
187.21 KB

Ah - so I found out some things but I could use some input to help me understand what *should* happen...

In function template_preprocess_file_link(), we set the file class, a sanitized mime type class and our own 'grouped' mime type class, prefixed file-- as per SMACSS. The grouping is done in file_icon_map().
You can see how a few files are shown by my css so far (including showing the actually classnames) here:
example classes

I feel we should only set icons by default on those classes with the file-- prefix. I don't know why - it just feels right.
To do this, we need to either:

  1. change ALL classes added in template_preprocess_file_link() to be prefixed file--. Mainly by adding this in line 1567 of file.module;
  2. add some further popular mime types, like application/pdf and it's siblings to the file_icon_map() function.

Which would people prefer?

rachel_norfolk’s picture

Assigned: rachel_norfolk » Unassigned
Status: Needs work » Needs review
FileSize
220.04 KB
3.12 KB
10.8 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 76,618 pass(es). View

Okay, I thought it would be easiest to just make a choice and see how people feel about it!

So, I have chosen a mixture of both choices above. I have:

  1. updated each css selector to use the form .file.file--some-filetype
  2. told template_preprocess_file_link() to prefix each raw mime type as file--mime-some-mimetype
  3. told template_preprocess_file_link() to check for dots in mime types and convert to dashes
  4. added some pdf mime types into file_icon_map().

So, the classes of some files now look like below:
example classes

As you can see, we now have a situation where the most popular files are painted with a pretty icon and themes can override these AND add further icons within a group should they wish to.

LewisNyman’s picture

FileSize
2.13 KB
2.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-49.patch. Unable to apply patch. See the log in the details link for more information. View
+++ b/core/modules/file/css/file.formatter.generic.css
@@ -0,0 +1,67 @@
+.file.file--general {
+  background-image: url(../icons/application-octet-stream.png);
+}
+.file.file--package-x-generic {
+  background-image: url(../icons/package-x-generic.png);
+}

Thanks! We can actually just remove the .file class from the selectors now, so just .file--general etc.

I've updated the patch so others can review the PHP.

Status: Needs review » Needs work

The last submitted patch, 49: use_css_for_file_icons-2236855-49.patch, failed testing.

LewisNyman’s picture

Status: Needs work » Needs review
FileSize
10.72 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855-51.patch. Unable to apply patch. See the log in the details link for more information. View

Whoops

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

<3 This patch:)

Gave it a read through and a peek on simplytest.me. Great work on this all!

rachel_norfolk’s picture

Working for me :-)

Oh - and I have made a Change Record - see https://www.drupal.org/node/2342157

The last submitted patch, 46: use_css_for_file_icons-2236855-45.patch, failed testing.

rachel_norfolk’s picture

Assigned: Unassigned » rachel_norfolk

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 51: use_css_for_file_icons-2236855-51.patch, failed testing.

lauriii’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
8.89 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,014 pass(es). View
lauriii’s picture

Assigned: rachel_norfolk » Unassigned

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 58: use_css_for_file_icons-2236855-58.patch, failed testing.

Status: Needs work » Needs review
lauriii’s picture

Status: Needs review » Reviewed & tested by the community

putting back to rtbc

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 58: use_css_for_file_icons-2236855-58.patch, failed testing.

Status: Needs work » Needs review
lauriii’s picture

Status: Needs review » Reviewed & tested by the community

back to rtbc again

rachel_norfolk’s picture

Status: Reviewed & tested by the community » Needs work

It's the issue that just keeps on giving, eh? Sorry, the css file has gone AWOL.

(this does kind of make me think we should have a test to check that files referenced in the yml files do actually exist)

rachel_norfolk’s picture

Status: Needs work » Needs review
FileSize
10.73 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855_67.patch. Unable to apply patch. See the log in the details link for more information. View
1.84 KB

Including the css... (I hope!)

Ieva Uzule’s picture

The patch applies cleanly and does exactly what it should.

Screenshot before:
Screenshot_before

Screenshot after:
Screenshot_after

Ieva Uzule’s picture

sorry, accidental duplicate..

Ieva Uzule’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 67: use_css_for_file_icons-2236855_67.patch, failed testing.

stefank’s picture

FileSize
12.88 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,374 pass(es). View

Here is a reroll #67.

stefank’s picture

Status: Needs work » Needs review
stefank’s picture

FileSize
10.75 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch use_css_for_file_icons-2236855_74.patch. Unable to apply patch. See the log in the details link for more information. View

Another try, as accidentally included interdiff in the patch.

alexpott’s picture

One issue with this approach is that file icons will not appear if someone prints the page.

alexpott’s picture

We could instead change this to something like

.file--general:before {
  content: url(../icons/application-octet-stream.png);
}

But I'm not sure how that plays out for accessibility.

LewisNyman’s picture

hm, is that behaviour desirable? I know they were being printed before but that feels like an unintentional side effect of the previous technique rather than an intentional decision.

All icons should be presentational only and implemented with background-image and from what I can see they are consistently implemented with background-image throughout core. If browsers choose not to print presentational images then that is their decision, let's not make an exception here.

This feels like a simple decision that sticks to the rest of core. Leaving on needs review for a day or so so others can weigh in.

rachel_norfolk’s picture

...and people can set their browsers to print background images, if they so choose.

LewisNyman’s picture

@rachel_norfolk I didn't know that, I found some documentation.

I've written the change record, we need someone to check it over before we RTBC: https://www.drupal.org/node/2358511

rachel_norfolk’s picture

Status: Needs review » Reviewed & tested by the community

Change record looks good, certainly better than mine!

Back to RTBC because this has been reviewed very thoroughly now and it's a *really* beneficial change. :-)

effulgentsia’s picture

Assigned: Unassigned » alexpott

Assigning to alexpott, to give him a chance to verify that his concerns have been adequately answered.

alexpott’s picture

Assigned: alexpott » Unassigned

I'm not that fussed about printing a web page - I just wanted to make sure that the concern with the new approach was discussed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 74: use_css_for_file_icons-2236855_74.patch, failed testing.

lauriii’s picture

Issue tags: +Needs reroll
stefank’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
10.78 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,077 pass(es). View

Here is an reroll.

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
445.14 KB

Thanks for the rerolls. I've manually tested the patch again just to make sure. I think we are back to RTBC

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks like alexpott's concern was resolved, and that answer is fine for me as well.

Committed/pushed to 8.0.x, thanks!

  • catch committed 0413834 on 8.0.x
    Issue #2236855 by rachel_norfolk, stefank, ngocketit, lauriii,...

Status: Fixed » Closed (fixed)

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