Bartik has unused shortcut-related code

Committed.

Unless I'm missing something, this isn't a setting that can be toggled. There is no way to turn this back on that I can see.

There is however, quite a bit of code left over from it that now does nothing, and will therefore confuse people.

This is interesting. At first I though "good catch!" — let's remove the code. But after looking at is a bit more, I don't know. People can turn on the shortcuts by toggling the 0 back to 1 in the last line of the .info file. I know this use case will be uncommon, but I can see people doing this. Perhaps. Do we want to leave the code for them?

As Bartik is now, if you turn on shortcut links (by editing the .info file), a node page looks like this:

If we run this patch, are remove this code, and someone turns on shortcuts the page looks like this:

So this patch is a good idea in the interest of code purity. Assuming no one ever turns on shortcuts.
Not committing this patch is a good idea in the interests of edge-case usability.

Meanwhile, I ran the patch. It messes up the way the shortcuts look (if they are turned on.) Otherwise it's awesome. Doesn't hurt anything. Doesn't affect anything, actually. Cleans up code.

I would RTBC the patch based on it's own merits.... but I'm having a debate in my head, and honestly usability beats code purity. So I'm not going to RTBC this.

It would make sense to keep the code if there was a way to turn this on in the UI, but there isn't. This is just my opinion after spending a good 15-20 minutes wondering what WTF this code is supposed to be doing in template.php and looking for a way to turn it on in the UI, all the while getting confused by the Favicon settings that are referred to as "Shortcut link" as well. That is the reason why I think "code purity" is more important than an edge-case that requires hacking core, using Strongarm or a variable_set() to make work.

It looks like this code is there as a workaround for #674394: The add/remove shortcut buttons look bad and don't appear in most themes besides Seven? I guess there is some logic to keeping it around until that issue is fixed, but I agree it wouldn't hurt a whole lot to remove it either.

If we remove that code, we should remove this line from the info file also:

settings[shortcut_module_link] = 0

It seems like its only purpose is to encourage people to experiment with turning it on...

Let's review this patch first: #674394-43: The add/remove shortcut buttons look bad and don't appear in most themes besides Seven

(If that goes in, then we won't want to remove this, because the code will actually be used.)

The patch in #3 still applies. However, the issue title was outdated :)

At this point I'd kind of lean towards leaving it in, even though #674394: The add/remove shortcut buttons look bad and don't appear in most themes besides Seven (the issue I linked to above) isn't likely to get fixed in Drupal 7 anymore. The code is harmless and it could help someone who wanted to turn the shortcuts on (after all, Bartik is supposed to be usable as an admin theme).

Either way, I think this is "needs work", per #8. If we're going to remove it, we should remove all of it (including the line in the info file).

Correct - this code was left in so people could experiment with turning it on and that it would work as an admin theme - this makes me cagey about rushing to remove it, however in light of #1164782: The icon to add something to shortcuts wasn't clearly discoverable. it looks like this whole shortcuts-icon concept needs rethinking.

So:

1. Lets postpone this issue awaiting an outcome on #1164782
2. If we remove it, then we remove all of it as per #8.

This issue needs updating and being moved from postponed as it has been for 3 years! I am finding it hard to follow but I can see that #1164782: The icon to add something to shortcuts wasn't clearly discoverable. has been moved down to D7.

This is not an issue in drupal 8 -- still mentioned in drupal 7 -- changing version to 7.37