Opened 14 years ago
Closed 14 years ago
#11504 closed defect (bug) (fixed)
PHP notices when publishing with QuickPress
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | Warnings/Notices | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
See attached screenshot taken after posting from QuickPress. There are also errors when saving a draft from QuickPress; though less of them.
The post is published anyway.
WP r12462, PHP 5.2.5
Attachments (4)
Change History (22)
#1
@
14 years ago
- Summary changed from PHP errors when publishing with QuickPress to PHP notices when publishing with QuickPress
#3
follow-up:
↓ 5
@
14 years ago
- Milestone changed from 3.0 to 2.9.1
Westi, please defined "published correctly" in this ticket's report context. Please provide the steps which drove you into this conclusion. Looks pretty much to unchecked indezies to me.
Referenced: #10758
#5
in reply to:
↑ 3
@
14 years ago
Replying to hakre:
Westi, please defined "published correctly" in this ticket's report context. Please provide the steps which drove you into this conclusion. Looks pretty much to unchecked indezies to me.
Referenced: #10758
My understanding of this ticket was that the post published fine but if WP_DEBUG was enabled then you see those notices.
If the functionality works correctly but there are notices shown with WP_DEBUG enabled while the notices are not desirable and we would fix them for the next major release I would advocate against making the changes in a point release unless the notices cause a security/blocker bug
#6
@
14 years ago
I'm pretty shure that I would have run over this as well in #10758 if Quickpress would have been used on the blog that was providing the data on unchecked indezies / falsly referenced variables. Since we already put focus of such warnings to get them fixed in 2.9 the consequence should be to fix the missed ones as well. Maybe not 2.9.1 but while there is no 2.9.2 milestone yet, I prefer 2.9.1 over 3.0. We can shift tickets with a not-high prio (like this one with normal) then to the next milestone (2.9.2) when 2.9.1 is released.
I will now try reproduce this. The point to WP_DEBUG is misleading because core code must run without any warning so that WP_DEBUG is a useable feature.
#7
@
14 years ago
I can confirm the problem, warnings because of unset variables while using Quickpress. Patch does fix the problem.
#9
follow-up:
↓ 10
@
14 years ago
Is there any reason not to fix this in 2.9, regardless of whether it is a blocker or not? It is still a bugfix and it has a patch.
(Not sure exactly what the criteria for eligibility for fixing in a point release is - I thought any bug could be fixed, but no new features. Is that wrong?)
#10
in reply to:
↑ 9
@
14 years ago
- Milestone changed from 2.9.1 to 3.0
Replying to caesarsgrunt:
Is there any reason not to fix this in 2.9, regardless of whether it is a blocker or not? It is still a bugfix and it has a patch.
(Not sure exactly what the criteria for eligibility for fixing in a point release is - I thought any bug could be fixed, but no new features. Is that wrong?)
Read the milestone description:
"This Milestone is only for security and blocker level bugs that exist in 2.9 after it is released."
#11
@
14 years ago
Thanks for clarification. Please apply it to the current head branch which will become 3.0.
#12
@
14 years ago
hakre:
I was using the patch when testing QuickPress for #10680. Looks good -- two things:
The line that sets $previous_status would now have two ternary operations, all for a function's second argument. It would probably be better as multiple lines.
Also,
if ( 'page' == ( isset($_POST['post_type']) ? $_POST['post_type'] : null ) )
Should probably be:
if ( isset($_POST['post_type']) && 'page' == $_POST['post_type'] )
That's all I got.
#13
@
14 years ago
Ok.
QuickPress will never pass $user_ID, but setting it to null is not correct. Root of the problem: it should instead set it to $GLOBALSuser_ID?, or pass it via POST to begin with.
Patching.
#14
@
14 years ago
I only set to NULL because that's the same value as if you read an unset variable by accident. So it does not change the behaviour compared to pre-patched. That's all, I know it's a bit akward.
Just reviewed your patch, why have you removed the cast to int in front of $post_data['user_ID'];
Line 40 / 43?
Thanks for your review as well, I appreceate the feedback.
#16
follow-up:
↓ 17
@
14 years ago
@nacin: isn't it possible for $post_data[ID] to be set with a value of zero on drafts? I'd suggest !empty() instead of isset() for the checks when setting the post_id variable.
#17
in reply to:
↑ 16
@
14 years ago
Replying to Denis-de-Bernardy:
isn't it possible for $post_data[ID] to be set with a value of zero on drafts? I'd suggest !empty() instead of isset() for the checks when setting the post_id variable.
Not sure. The code was already checking for isset(). Logic seems fine though for $post_id to be 0, I think. It's just using that to get the post's previous status.
If the post is published correctly then by default (i.e. with WP_DEBUG not enabled) these notices won't show and the 'expected' behaviour will be achieved.
Therefore this is not a severe enough issue to fix in a 2.9.1 release - moving to the 3.0 milestone where we will address it.