Skip to content
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

Refactor breadcrumbs class - fixes a number of bugs #833

Merged
merged 21 commits into from
Mar 17, 2014

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Mar 17, 2014

Issues fixed:

  • In the WPSEO -> Internal Links page text boxes we allow the entry of HTML, but this HTML was being output escaped when the breadcrumb was send to the screen resulting in <span>'s and such
  • The return value of yoast_breadcrumbs() used to be either the breadcrumb string or true, with v1.5 it became string or null. Fixed to be the same as before again.
    This will hopefully solve at least some issues with themes.
  • With the changes to v1.5, the defaults for some of the textual breadcrumb settings are added dynamically, but empty strings are allowed. This caused issues for people who left the text fields empty on purpose relying on the defaults. Fixed it by adding the defaults to the option if the fields were left empty - but only for people upgrading from versions < 1.5.3, so intentionally left blank fields after that will be respected.
    This will also solve some issues where people didn't see the breadcrumb because the default 'Home' text for instance wasn't available. These issues may have incorrectly been blamed on theme integration.
  • Probably fixed: Last breadcrumb wasn't always determined correctly resulting in crumbs not being linked when they should have been. Haven't got a test case, so haven't been able to verify.
  • Also: Fixed: 404 date based breadcrumb and title creation could cause corruption of the $post object

@dannyvankooten This refactor should make it lot easier to write unit tests for this class. Hope you like it ;-)

Related issues #631, #795, #811, #816.

Also:

jrfnl added 17 commits March 15, 2014 16:53
…factor-breadcrumbs-class

Conflicts:
	readme.txt
With the changes to v1.5, the defaults for some of the textual breadcrumb settings are added dynamically, but empty strings are allowed.
This caused issues for people who left the fields empty on purpose relying on the defaults.
@jdevalk
Copy link
Contributor

jdevalk commented Mar 17, 2014

Can't automerge this and due to the number of commits I'd feel better if you merged yourself @jrfnl :)

…factor-breadcrumbs-class

Conflicts:
	readme.txt
jrfnl added a commit that referenced this pull request Mar 17, 2014
Refactor breadcrumbs class - fixes a number of bugs
@jrfnl jrfnl merged commit 8f1ef5a into Yoast:master Mar 17, 2014
@jrfnl
Copy link
Contributor Author

jrfnl commented Mar 17, 2014

Done, it was only the readme conflicting after the updates of this morning.

@jrfnl jrfnl deleted the Refactor-breadcrumbs-class branch March 17, 2014 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants