New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecating compatibility functions #4096

Open
Yoshi2889 opened this Issue Jun 4, 2017 · 17 comments

Comments

Projects
None yet
10 participants
@Yoshi2889
Contributor

Yoshi2889 commented Jun 4, 2017

Hopping through the codebase, I found the following things...

  • un_htmlspecialchars - PHP 5.1 and above includes htmlspecialchars_decode. I'm not sure how this function differs from it (it says it removes things but it just replaces things from reading the code).
  • html_to_bbc, bbc_to_html, legalise_bbc (and sort_array_length) - I know the notes say that they shouldn't be removed due to mod compatibility, however mods have to be updated for 2.1 anyway so authors could move to a different implementation.
  • theme_postbox - Used in SMF 1.1. Need I say more :x
  • smf_setcookie has a note that it can be removed since SMF requiers PHP >= 5.3.8 now.
@live627

This comment has been minimized.

Show comment
Hide comment
@live627

live627 Jun 7, 2017

Contributor

Huh, I always thought un_htmlspecialchars() and htmlspecialchars_decode() did the same thing.

Contributor

live627 commented Jun 7, 2017

Huh, I always thought un_htmlspecialchars() and htmlspecialchars_decode() did the same thing.

@Sesquipedalian

This comment has been minimized.

Show comment
Hide comment
@Sesquipedalian

Sesquipedalian Jun 8, 2017

Member

un_htmlspecialchars - PHP 5.1 and above includes htmlspecialchars_decode. I'm not sure how this function differs from it (it says it removes things but it just replaces things from reading the code).

Yeah, they look the same to me. We should probably keep it around for old mods, but we could replace the current content of the function with a simple return htmlspecialchars_decode($string).

html_to_bbc, bbc_to_html, legalise_bbc (and sort_array_length) - I know the notes say that they shouldn't be removed due to mod compatibility, however mods have to be updated for 2.1 anyway so authors could move to a different implementation.

In theory the mods should be updated, sure, but I doubt they all will. It doesn't hurt us to keep these functions around for now.

theme_postbox - Used in SMF 1.1. Need I say more :x

No, you needn't. Death to it!

smf_setcookie has a note that it can be removed since SMF requiers PHP >= 5.3.8 now.

Ditto for this one.

Member

Sesquipedalian commented Jun 8, 2017

un_htmlspecialchars - PHP 5.1 and above includes htmlspecialchars_decode. I'm not sure how this function differs from it (it says it removes things but it just replaces things from reading the code).

Yeah, they look the same to me. We should probably keep it around for old mods, but we could replace the current content of the function with a simple return htmlspecialchars_decode($string).

html_to_bbc, bbc_to_html, legalise_bbc (and sort_array_length) - I know the notes say that they shouldn't be removed due to mod compatibility, however mods have to be updated for 2.1 anyway so authors could move to a different implementation.

In theory the mods should be updated, sure, but I doubt they all will. It doesn't hurt us to keep these functions around for now.

theme_postbox - Used in SMF 1.1. Need I say more :x

No, you needn't. Death to it!

smf_setcookie has a note that it can be removed since SMF requiers PHP >= 5.3.8 now.

Ditto for this one.

@Antes

This comment has been minimized.

Show comment
Hide comment
@Antes

Antes Jun 8, 2017

Contributor

@Sesquipedalian backward compat broken for a long time so anything unnecessary should better be dead in sense of consistency.

But the thing is, anything removed from SMF (Source) should be noted in wiki so mod authors can update their mods far more easily.

Contributor

Antes commented Jun 8, 2017

@Sesquipedalian backward compat broken for a long time so anything unnecessary should better be dead in sense of consistency.

But the thing is, anything removed from SMF (Source) should be noted in wiki so mod authors can update their mods far more easily.

@Yoshi2889

This comment has been minimized.

Show comment
Hide comment
@Yoshi2889

Yoshi2889 Jun 8, 2017

Contributor

We need a wiki page with all the developer changes anyway, like hooks added/changed and stuff.

Contributor

Yoshi2889 commented Jun 8, 2017

We need a wiki page with all the developer changes anyway, like hooks added/changed and stuff.

@colinschoen

This comment has been minimized.

Show comment
Hide comment
@colinschoen

colinschoen Jun 8, 2017

Member

i'll make a post in the doc team boards to see if they can get a page template sorted for us and we can start updating it.

Member

colinschoen commented Jun 8, 2017

i'll make a post in the doc team boards to see if they can get a page template sorted for us and we can start updating it.

@frandominguez03

This comment has been minimized.

Show comment
Hide comment
@frandominguez03
Member

frandominguez03 commented Jun 8, 2017

@colinschoen

This comment has been minimized.

Show comment
Hide comment
@colinschoen

colinschoen Jun 8, 2017

Member

Specifically a page for developers and mod authors that shows the changes in functions and available hooks.

Member

colinschoen commented Jun 8, 2017

Specifically a page for developers and mod authors that shows the changes in functions and available hooks.

@Arantor

This comment has been minimized.

Show comment
Hide comment
@Arantor

Arantor Aug 24, 2017

Contributor

The behaviour is more evident in a mixed encoding environment but it is not a 1:1 change because the behaviours differ when considering the behaviour of nbsp decoding. Using the internal function yields a real nbsp character (U+00A0) while this function yields U+0020 which in some of the places it is used, could be significant, e.g. with posts and the preservation of spaces that is carried out there.

Contributor

Arantor commented Aug 24, 2017

The behaviour is more evident in a mixed encoding environment but it is not a 1:1 change because the behaviours differ when considering the behaviour of nbsp decoding. Using the internal function yields a real nbsp character (U+00A0) while this function yields U+0020 which in some of the places it is used, could be significant, e.g. with posts and the preservation of spaces that is carried out there.

@Yoshi2889

This comment has been minimized.

Show comment
Hide comment
@Yoshi2889

Yoshi2889 Aug 24, 2017

Contributor

@Arantor I suppose you're talking about the un_htmlspecialchars function?

Contributor

Yoshi2889 commented Aug 24, 2017

@Arantor I suppose you're talking about the un_htmlspecialchars function?

@Arantor

This comment has been minimized.

Show comment
Hide comment
@Arantor

Arantor Aug 24, 2017

Contributor

Yup, and changing it out is not consequence free.

Contributor

Arantor commented Aug 24, 2017

Yup, and changing it out is not consequence free.

@Sesquipedalian

This comment has been minimized.

Show comment
Hide comment
@Sesquipedalian

Sesquipedalian Aug 25, 2017

Member

Using the internal function yields a real nbsp character (U+00A0) while this function yields U+0020

Good to know. Thanks, @Arantor.

Member

Sesquipedalian commented Aug 25, 2017

Using the internal function yields a real nbsp character (U+00A0) while this function yields U+0020

Good to know. Thanks, @Arantor.

Yoshi2889 added a commit to Yoshi2889/SMF2.1 that referenced this issue Aug 25, 2017

Update #4096
Slightly changes the login treshold string to include the member name.

Signed-off-by: Yoshi2889 <rick.2889@gmail.com>

Yoshi2889 added a commit to Yoshi2889/SMF2.1 that referenced this issue Aug 25, 2017

Update #4096
Add 'login' error category, and include login_threshold_brute_fail

Signed-off-by: Yoshi2889 <rick.2889@gmail.com>

Yoshi2889 added a commit to Yoshi2889/SMF2.1 that referenced this issue Aug 25, 2017

Update #4096
Include login_threshold_fail in login error category.

Signed-off-by: Yoshi2889 <rick.2889@gmail.com>

@Yoshi2889 Yoshi2889 referenced this issue Aug 25, 2017

Merged

Fix #4098 #4266

@Yoshi2889

This comment has been minimized.

Show comment
Hide comment
@Yoshi2889

Yoshi2889 Aug 25, 2017

Contributor

smf_setcookie actually has some hook stuff going on, so we might not want to remove this. It also calls setcookie() natively.

Contributor

Yoshi2889 commented Aug 25, 2017

smf_setcookie actually has some hook stuff going on, so we might not want to remove this. It also calls setcookie() natively.

@Yoshi2889

This comment has been minimized.

Show comment
Hide comment
@Yoshi2889

Yoshi2889 Aug 25, 2017

Contributor

Also, do we agree on removing the bbc and html functions now?

Contributor

Yoshi2889 commented Aug 25, 2017

Also, do we agree on removing the bbc and html functions now?

@Arantor

This comment has been minimized.

Show comment
Hide comment
@Arantor

Arantor Aug 26, 2017

Contributor

The problem is that 2.1 really does pitch itself as trying to be compatible with 2.0 mods, to the point of offering to help install them. For a modest incremental update as originally envisioned, this would have worked - templates didn't even need major overhauls for the most part.

But I think yes, it's probably at the point where they should go now. theme_postbox definitely needs to die already.

Contributor

Arantor commented Aug 26, 2017

The problem is that 2.1 really does pitch itself as trying to be compatible with 2.0 mods, to the point of offering to help install them. For a modest incremental update as originally envisioned, this would have worked - templates didn't even need major overhauls for the most part.

But I think yes, it's probably at the point where they should go now. theme_postbox definitely needs to die already.

@Arantor

This comment has been minimized.

Show comment
Hide comment
@Arantor

Arantor Aug 27, 2017

Contributor

Oh, and html_to_bbc is in use in the manage boards area so you can't just remove it.

Contributor

Arantor commented Aug 27, 2017

Oh, and html_to_bbc is in use in the manage boards area so you can't just remove it.

@MissAllSunday

This comment has been minimized.

Show comment
Hide comment
@MissAllSunday

MissAllSunday Sep 5, 2017

Contributor

Yes, html_to_bbc is still used to allow basic BBC usage inside board description, I believe categories too.

Contributor

MissAllSunday commented Sep 5, 2017

Yes, html_to_bbc is still used to allow basic BBC usage inside board description, I believe categories too.

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Sep 6, 2018

Member

Looking at the code and all the usage of un_htmlspecialchars, I would say at this point in 2.1 life cycle it could cause a BC issue if we change it. Too much testing would need to be done to ensure that we didn't break something with this change.

Thoughts?

Member

jdarwood007 commented Sep 6, 2018

Looking at the code and all the usage of un_htmlspecialchars, I would say at this point in 2.1 life cycle it could cause a BC issue if we change it. Too much testing would need to be done to ensure that we didn't break something with this change.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment