Problem/Motivation
go to /batch and you'll get a 403 and a php warning:
Cannot modify header information - headers already sent by (output started at /var/www/html/includes/common.inc)
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff_5-6.txt | 2.11 KB | poker10 |
| #10 | 3138348-10.patch | 1.39 KB | poker10 |
| #10 | 3138348-10_test-only.patch | 1013 bytes | poker10 |
| #5 | drupal-batch-403-warning-3138348-5.patch | 1.89 KB | awset |
| #5 | interdiff_2_5.txt | 1.49 KB | awset |
Comments
Comment #2
mikeytown2 commentedComment #3
jungle@mikeytown2, thanks for filing this.
Applying IS template, tagging "Needs tests" and "Bug Smash Initiative".
Comment #4
kristen polThanks for the issue and patch. I've tested as follows:
/batch/batchMoving back to "needs work" for tests.
Comment #5
awset commentedHi All,
thanks for the good work.
Adding a test for the patch provided by mikeytown2.
I also added the interdiff for the same.
this need a review.
thanks
Comment #6
poker10 commentedThanks @awangsetyawan for the test. I have tweaked it a little bit to remove the need of the new method (assertNoLogMessage()), so we can keep the code cleaner.
Adding test-only patch and regular patch to see if testbot will be happy. If yes, I think this can be set as RTBC, as the patch is trivial and it is confirmed also by manual testing.
Comment #8
poker10 commentedAs I have mentioned in #6, I am setting this as RTBC, because patch was unchanged and confirmed that is is working before. Tests are passing / failing as expected too.
Comment #9
kristen pol@poker10 Thanks for the updated patch. Please create an interdiff when making changes:
https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa...
Since you updated the patch, you can't mark this RTBC so moving back to Needs review.
Comment #10
poker10 commentedThanks @Kristen Pol for pointing this out. It seems like I made an unintentional mistake here - I did not mean to RTBC my own patch. Reuploading the patch from #6 again with an interdiff, which was missing here.
Comment #11
kristen polThanks @poker10 :)
Comment #13
mcdruid commentedThere are several other places in core where we call
drupal_exit()afterdrupal_access_denied()for example:Patch looks good, thanks!
Comment #14
kristen polWhat’s RTBM? Curious:)
Comment #15
mcdruid commentedRTBM is Ready To Be Merged :) so approved by one maintainer for another to commit.
It's mentioned in #3093258: [policy, no patch] Proposed Drupal 7 contributor / maintainer workflow which never really got beyond a draft, but we've been following it pretty much for a while in D7.
Comment #16
kristen polOh! Very cool. I wasn’t familiar. Thanks:)
Comment #18
poker10 commentedThanks everyone who contributed!