MyBB Community Forums
[Pushed] Editing announcement is broken when global variable "$announcement" - Printable Version

+- MyBB Community Forums (https://community.mybb.com)
+-- Forum: Development (https://community.mybb.com/forum-161.html)
+--- Forum: MyBB 1.8 Development (https://community.mybb.com/forum-165.html)
+---- Forum: 1.8 Bugs and Issues (https://community.mybb.com/forum-157.html)
+----- Forum: Pushed (https://community.mybb.com/forum-183.html)
+----- Thread: [Pushed] Editing announcement is broken when global variable "$announcement" (/thread-163656.html)



Editing announcement is broken when global variable "$announcement" - Jones H - 2014-12-02

If a plugin uses a global variable named "$announcement" the modcp edit announcement part will thrown the error "The specified announcement doesn't exist". That's because of this if. The reason for the if is to avoid that the announcement is loaded multiple times (it's already loaded in do_edit_announcement normally). However it fails when the variable is already in use. IMHO we should add a check for request_method == "post".


RE: Editing announcement is broken when global variable "$announcement" - Jones H - 2015-06-25

Bump


RE: Editing announcement is broken when global variable "$announcement" - Destroy666 - 2015-06-28

Either the linked lines are outdated or I don't understand what's wrong.

EDIT: and if you mean the 2 conditionals above, which you most likely do - won't adding request_method == "post" to the 1st check skip the query when you're accessing the action directly, which would break it completely? Unless you meant adding that condition as an alternative rather than &&:
if(!isset($announcement) || $mybb->request_method == 'post')
but then this optimisation is kind of pointless since the query will be always executed.

EDIT2: the same thing happens probably in many places btw, I just remembered that I accidentally broke quick reply in one of my plugins by similarly globalizing $post['xxx'] in a global hook. This line fired off: https://github.com/mybb/mybb/blob/feature/newreply.php#L86 Not sure if that's something we can/should fix everywhere.


RE: Editing announcement is broken when global variable "$announcement" - Jones H - 2015-06-29

Yeah, meant the "isset" - if the request method is post, the variable/announcement is loaded already. If you're accessing it directly (without a post request) the query is always executed. So the if would be:
if(!isset($announcement) || $mybb->request_method != 'post')

Don't know about other places but IMHO we should fix those cases to - "announcement" is a very common variable name, especially for announcement managers (we have at least 3 of them on the mods site).

Edit: Probably even better to check for $mybb->input['action'] != 'do_edit_announcement'.


RE: Editing announcement is broken when global variable "$announcement" - Destroy666 - 2015-06-29

(2015-06-29, 06:32 AM)JonesĀ H Wrote: Edit: Probably even better to check for $mybb->input['action'] != 'do_edit_announcement'.

That would surely not work because the action is edit_announcement at that point.

if(!isset($announcement) || $mybb->request_method != 'post') 
seems ok though.


RE: Editing announcement is broken when global variable "$announcement" - Jones H - 2015-06-29

Ah, yeah, forgot that we change that always...


RE: Editing announcement is broken when global variable "$announcement" - Jones H - 2015-07-02

Hi,

Thank you for your report. We have pushed this issue to our Github repository for further analysis where you can track our commits and progress with fixing this bug. Discussions regarding this bug may also take place there too.

Follow this link to visit the issue on Github: https://github.com/mybb/mybb/issues/2129

Thanks for contributing to MyBB!

Regards,
The MyBB Group