Respect Genesis breadcrumb enable setting ?

[jrfnl]

in the yoast_breadcrumb() function we check for our own 'enable' setting. We may want to also check for the Genesis framework breadcrumb enable setting.

Apparently pre-1.5 people could enable in Genesis and would then not have to go to WPSEO to enable as well, while in 1.5 they need to enable in both. This is causing some confusion.

[wpperform]

To clarify, it seems that 1.5 is taking over Genesis breadcrumbs. It is not merely a matter of enabling breadcrumbs in 1.5 (if by that you mean checking the box). One has to enable breadcrumbs and then configure breadcrumbs in 1.5. Most Genesis theme developers would configure breadcrumbs in Genesis, not WP SEO, and this worked fine pre 1.5.

Therefore, the enable checkbox 1.5 should not do anything to breadcrumbs when it is not checked.

In 1.5, checking the enable checkbox does not restore the Genesis breadcrumb functionality. It only puts an unlinked restatement of the post title in place of the breadcrumb.

[jrfnl]

Related #795 - includes sample code of how to fix breadcrumb issues in themes.

[jrfnl]

To clarify, it seems that 1.5 is taking over Genesis breadcrumbs.

@wpperform Actually no, the Genesis check for whether to use WPSEO breadcrumbs or use their own is outdated since v1.5. Their check needs adjusting and then things should go back to 'normal' ;-)
Genesis doesn't check whether WPSEO breadcrumbs are enabled, they check whether the function to output them is available. The change in v1.5 vs pre-1.5 is that that function used to only be available if breadcrumbs was enabled, now the function is always available.
The WPSEO function however does check the setting and won't output anything if it's unchecked, leaving you without breadcrumbs.

[jdevalk]

Pinging @nathanrice, I think this should actually be fixed in Genesis. If we "fix" it, people could get a warning due to the fact that the function doesn't exist when it's included in their theme.

However, can we get a workaround going for now so we can give the expected behavior to people?

[jrfnl]

@jdevalk

1. Yes, this should be fixed in Genesis, but the issue is that people apparently didn't used to have to also check the WPSEO breadcrumb checkbox and it seems that now they do need to.
2. People should still use the function_exists() check, have a look at the example code I provided in Genesis default breadcrumbs overridden #795 (comment) for theme owners showing how to better implement using WPSEO breadcrumbs with a theme native fallback.
3. Not sure about how to get a work-around going as I'm unfamiliar with the Genesis code. Any takers ?

[wpperform]

Here is one possible fix:

In line 61 of /genesis/lib/functions/breadcrumb.php, change ...

This:

elseif ( function_exists( 'yoast_breadcrumb' ) ) {
    yoast_breadcrumb( '<div class="breadcrumb">', '</div>' );
}

To:

elseif ( function_exists( 'yoast_breadcrumb' ) ) {
    $options = get_option( 'wpseo_internallinks' );
    if ( $options['breadcrumbs-enable'] === false ) {
        genesis_breadcrumb();
        return;
    }
    yoast_breadcrumb( '<div class="breadcrumb">', '</div>' );
}

[jrfnl]

@wpperform I appreciate you are trying to help, but this is not helping. There is already better example code available in the other thread. Your code depends on WPSEO never renaming any options, the example code I posted in the other thread does not.

[wpperform]

@jrfnl -

#1 I never read the comments in the other thread since your request for "any takers" was after you posted a solution there and after your link in this thread.

#2 The first solution you posted depends - just like mine - on WPSEO never renaming options. The second solution you posted there doesn't work as posted; mine does. It's not clear if you tested your solution on the current release of Genesis; I did test mine.

#3 Given that the discussion of this issue continued in 2 threads after connecting the 2, it might make sense to close one so that it's easier to follow things.

[jrfnl]

@wpperform -

1. sorry, maybe I should have been clearer. My 'any takers' comment was meant in answer to 'can we get a workaround going for now so we can give the expected behavior to people?', i.e. for a work-around within WPSEO to sort this out until Genesis has fixed this... not for a fix for Genesis itself.

2. The second one is clearly marked the preferred solution. The fact that it 'doesn't' work is not so strange as the theme specific function needs to be added as is clearly noted in the code. I guess if you do that, it will work. There is no reason why it shouldn't.

3. If you actually read my initial report you can see that it's really about something different which is why both threads should remain open. Issue Genesis default breadcrumbs overridden #795 deals with themes needing to change their code and how to do so. This issue is about a change in expected behaviour and whether we need to provide a work-around for that within WPSEO.

   Apparently pre-1.5 people could enable in Genesis and would then not have to go to WPSEO to enable as well, while in 1.5 they need to enable in both. This is causing some confusion.

The fact that you've turned this thread into something else is really not helping to keep the issues clear.

[nathanrice]

So, just to clarify what happens in WPSEO ...

If the user pastes the call to yoast_breadcrumb() in their theme, they still have to "enable" it with an option?

[wpperform]

@nathanrice - Ignoring any theme changes, if you activate WP SEO in a Genesis theme and enable Genesis breadcrumbs, they don't show up even if WP SEO breadcrumbs are disabled. That's because the if/then conditional aborts and the genesis_breadcrumb() function never executes. If you enable WP SEO breadcrumbs, the WP SEO breadcrumbs show up and have to be configured; the Genesis breadcrumbs effectively never show up.

@jrfnl -

1. Your "any takers" comment was unclear. You said "this should be fixed in Genesis" and I provided a fix based on your comment that you were "unfamiliar with the Genesis code".
2. There's no need to be condescending. It's pretty clear from the code you provided that the theme specific function needs to be included. Even with that, it doesn't work. You're a talented programmer, so I think if you look at the Genesis code and try your solution on a Genesis site, you'll see why it doesn't work.
3. You are right that issue Genesis default breadcrumbs overridden #795 and this one are slightly different, and you may be of the view that Genesis default breadcrumbs overridden #795 is about fixing Genesis and this issue is about fixing WP SEO. I get that. But both your previous comment and @jdevalk's comment indicated this should be fixed in Genesis, which is what I offered. Further, based on my understanding of Genesis 2.0.2, I don't think it is possible for you to implement a workaround in WP SEO that allows a Genesis user the option of using either Genesis breadcrumbs or WP SEO breadcrumbs. You can disable WP SEO breadcrumbs for Genesis users, but that might have bad consequences.
4. You previously said I was "not helping" but I respectfully disagree. I think you misunderstand the problem. You expressed the goal of trying to protect the Genesis code from changes to WP SEO, and that's a great goal. However, the fix in WP SEO that you're seeking in this thread just puts that burden on WP SEO as opposed to Genesis. Genesis broke because its code relied on checking for the existence of a function. A workaround is to check for the value of WP SEO option, but that will break if the WP SEO option changes - just like the WP SEO workaround will break if Genesis functions or options change. For the 2 things to play nicely together, Genesis & WP SEO have to recognize that something (function name or option name) can't change without breaking sites. In this case, since WP SEO updated, I think it was WP SEO's responsibility to take this into account. I am not saying that to point fingers or assign blame. I'm saying that so that the WP SEO team can learn from the experience and appreciate that changes to how functions operate or option names can break sites.

I hope that helps. I'm not trying to turn this thread into anything other than a) getting to a resolution of the problem and b) reducing the chance it occurs in the future.

[nathanrice]

Ignoring any theme changes, if you activate WP SEO in a Genesis theme and enable Genesis breadcrumbs, they don't show up even if WP SEO breadcrumbs are disabled.

Is this new behavior? Admittedly, it's been a while since I've done anything with breadcrumbs, but if I remember correctly, it used to just output the breadcrumb (the function accepted params) when the function was called. Of course, this was back when it was a breadcrumb plugin, not part of the SEO plugin.

Is there a specific reason why a person who pastes code into their theme files needs to further "configure" the breadcrumbs on an options page? Wanting to display breadcrumbs on their site is sort of implicit in the action of pasting the code in their theme, no?

[jrfnl]

@wpperform As far as I know, the Genesis people were informed in advance that v1.5 contained some changes they may need to test and adjust for. Independently of whether they were or were not actively approached, there's been a public beta-test period of over a six weeks before the release. No issues with the breadcrumbs change were reported to us during that period.

To be fair, I'd like to remove this whole discussion between us from the thread as it takes away from the actual issue.

[jrfnl]

@nathanrice For WPSEO, having to enable the breadcrumbs with the checkbox is not new behaviour, this has been the case since before this plugin made it's way to GitHub (nov 2012, I can't look further back).

Whether the behaviour in combination with Genesis is different, I don't know, you'd need to ask the Genesis people.

I agree with you that the checkbox is counter-intuitive and I've proposed removing it and just letting the function do it's thing, but I seem to remember there were compelling reasons why not to remove it, though I don't remember which. Issue #631 might contain more information as the original changes were discussed there.

[wpperform]

@nathanrice - New as of WP SEO 1.5, yes. See my comment here #816 (comment) on what fixes it in Genesis. I am sure there are cleaner ways to write that; it was just a quick proof of concept. What changed is that Genesis checks for the existence of a function, which didn't exist before unless a user was using WP SEO breadcrumbs. Now the function exists if the plugin is activated. Forget about pasting the code. If you simply enable the WP SEO breadcrumb options, breadcrumbs output, but without configuration, you get the unlinked post title where Genesis breadcrumbs would appear.

@jrfnl - As I stated above, WP SEO changed, so I think the burden is on WP SEO to recognize how previous users of WP SEO used it on their sites. WP SEO shouldn't issue updates that break sites. In this case, I do recognize that "break" is a bit strong. In this case, it wasn't up to Genesis to issue an update because WP SEO was changing.

I don't favor removing our discussion. It speaks for itself. I said that your code doesn't work; you insisted it does. If I'm wrong, I'll happily apologize. I said I don't think it is a productive effort to come up with a workaround for WP SEO for Genesis because I don't think one is possible without restricting WP SEO users, but I am happy to be proved wrong.

[wpperform]

@nathanrice After thinking about this more, I think the simplest solution is to simply comment out/remove lines 61-63 of /genesis/lib/functions/breadcrumb.php. (I tested this and it works.) That restores Genesis breadcrumb functionality, which is probably a bigger focus for the vast majority of Genesis users. If a Genesis user wants to use WP SEO breadcrumbs, it's easy enough to remove Genesis breadcrumbs and add the code required for WP SEO.

That is a fix that is easy for the SP support team to explain when needed until Genesis issues an update, and it's much less likely to be messed up by an average user. It also doesn't depend on any changes to WP SEO functions or options.

[tunjic]

Yes, commenting out lines 61-63 restored Genesis breadcrumbs and all my customizations.

/*  elseif ( function_exists( 'yoast_breadcrumb' ) ) {
        yoast_breadcrumb( '<div class="breadcrumb">', '</div>' );
    }
*/

[jdevalk]

The reason to leave the checkbox in was so with themes like Genesis people could choose to have either Genesis breadcrumbs or ours... If someone has a better solution to that problem now I'm all ears.

[wpperform]

Right now, users can't get Genesis breadcrumbs because of the 1.5 change combined with how Genesis does its check.

It is possible to modify Genesis to provide Genesis users with a choice; I already provided code for that above. I don't know if there is a way to modify WP SEO to do this without requiring a Genesis mod unless you can modify WP SEO so the check for function existence fails when the checkbox is not checked.

However since Genesis breadcrumbs can be removed with 1 line of code in a child theme, I think it is better route for Genesis to remove the WP SEO code, and that renders the checkbox virtually meaningless.

On Mar 16, 2014, at 3:24 PM, Joost de Valk notifications@github.com wrote:

The reason to leave the checkbox in was so with themes like Genesis people could choose to have either Genesis breadcrumbs or ours... If someone has a better solution to that problem now I'm all ears.

—
Reply to this email directly or view it on GitHub.

[nathanrice]

This has been addressed in the Genesis develop branch, and will be fixed in Genesis 2.1.0.