Related to issue #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional : color_scheme_form_submit() expects base_image to be set. In order to resolve #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional there needs to be a unit test. In addition, once #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional is committed, then Bartik will no longer need base image.

Solution: fix Bartik to remove this dependancy. I will submit a patch that will update Bartik so it can be used for testing and be free of this not-needed dependancy.

CommentFileSizeAuthor
#8 1366594--8.patch1.98 KBmike stewart
#6 1366594--6.patch1.07 KBmike stewart
#5 1366594--4.patch1.07 KBmike stewart
#1 1366594.patch686 bytesmike stewart

Comments

mike stewart’s picture

mike stewart commented
StatusFileSize
new 1366594.patch686 bytes

Proposed patch to use for testing #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional (and then ultimately use in core).

if you apply this patch without patch from 924322, you'll encounter:

Notice: Undefined index: base_image in color_scheme_form_submit() (line 338 of /var/projects/sandbox/drupal/core/modules/color/color.module).
Notice: getimagesize(): Read error! in color_scheme_form_submit() (line 339 of /var/projects/sandbox/drupal/core/modules/color/color.module).
Notice: Undefined index: base_image in color_scheme_form_submit() (line 400 of /var/projects/sandbox/drupal/core/modules/color/color.module).

once, applying patch from 924322, all is good again and dependency from Base image is complete.

mike stewart’s picture

mike stewart commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1366594.patch, failed testing.

Jeff Burnz’s picture

Jeff Burnz commented 
+++ b/core/themes/bartik/color/color.incundefined
@@ -119,14 +119,11 @@ $info = array(
-  // Reference color used for blending. Matches the base.png's colors.
+  // Reference color used for blending. ¶

White space

18 days to next Drupal core point release.

mike stewart’s picture

mike stewart commented
StatusFileSize
new 1366594--4.patch1.07 KB

Ok, if I understand, this patch failed because #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional has not yet been committed. Kind of the chicken and egg syndrome. Can't properly test 924322 for RTBC without a test case, but the only test case for D8 can't be committed until color module works differently.

However the failure made me realize the original patch wasn't complete, and I omitted deleting the two unneeded png files (base.png/preview.png) when I created the previous patch. Therefore I'm attaching a new patch, which I expect to fail for the same reason.

To help move this along, I spoke with someone else in the community and they suggested ALSO attaching a patch to 924322 which includes this patch as a "patch with test." So I'll do that, too.

mike stewart’s picture

mike stewart commented
Status: Needs work » Needs review
StatusFileSize
new 1366594--6.patch1.07 KB

oops, had a old browser tab and missed @jeff Burnz comment above... and last patch still has white space. so, hopefully this is it. I'll change status to see if it passes... but expect it to fail for reasons in #5

Status: Needs review » Needs work

The last submitted patch, 1366594--6.patch, failed testing.

mike stewart’s picture

mike stewart commented
Status: Needs work » Needs review
StatusFileSize
new 1366594--8.patch1.98 KB

Ok, this patch should be work. But still don't expect it to pass until this is committed: #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional.

Also, I'm not sure, but there may be some simple-tests that check for the too? (I'm not too familiar with how to fix any simple-tests if they exist). Help would be appreciated if this is the case.

Status: Needs review » Needs work

The last submitted patch, 1366594--8.patch, failed testing.

mike stewart’s picture

mike stewart commented
Status: Needs work » Needs review

Ok, this is basically the same patch as: http://drupal.org/node/924322#comment-5352650

Once that issue is RTBC and committed, this patch should pass.

Jean Gionet’s picture

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

this patch works perfectly if you first apply patch #7 from Devin Carlson.

great work Mike and Devin!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

If there's a dependency on the other patch, this one can't be rtbc.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented

#8: 1366594--8.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1366594--8.patch, failed testing.

steinmb’s picture

steinmb commented
Issue tags: +Needs issue summary update

Need an issue summary

markhalliwell’s picture

markhalliwell
Primary language English
commented
Issue tags: +Novice, +Needs reroll

.

David Hernández’s picture

David Hernández commented
Issue tags: -Needs reroll

Patch still applies, but #924322: color_scheme_form_submit() assumes color.inc provides keys that should be optional is not done yet.

botris’s picture

botris
Primary language Dutch
Location Creuse, France
commented
Issue summary: View changes
Issue tags: +Amsterdam2014

Looking into this.

botris’s picture

botris
Primary language Dutch
Location Creuse, France
commented
Status: Needs work » Closed (cannot reproduce)

Marking this closed. This was solved in the patch #924322-31: color_scheme_form_submit() assumes color.inc provides keys that should be optional which is committed.