Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2448 closed defect (bug) (fixed)

Disabling adminbar for logged out users - style tags incorrectly hooked

Reported by: hnla's profile hnla Owned by:
Milestone: 1.2.6 Priority: normal
Severity: Version:
Component: Core Keywords: adminbar, style tags
Cc: hnla

Description

Setting adminbar disabled for logged out views is controlled by adding display:none; to #wp-admin-bar via style tags, however the action hook is incorrect as it hooks into 'wp-footer' causing illegal and non valid page rendering with style tags rendered in the document body tags.

Solution:
In /bp-core/bp-core-cssjs.php ~line 182 change:

add_action( 'wp_footer', 'bp_core_override_adminbar_css' );

To:

add_action( 'wp_head', 'bp_core_override_adminbar_css' );

BP 1.2.4.1 and earlier?

Change History (9)

#1 @TobiasBg
12 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

Actually the function is useless and should be removed: #2024

#2 @hnla
12 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Sorry Tobias I've reopened this as I simply don't get the issue or rather I do and it doesn't really require a debate as in your ticket referenced. The fact is and regardless of whether the function is useless, it appears to remain there, is active and produces invalid DOM structure so regardless of the issues the simple fix IS to change it at the very earliest milestone and then debate how it ought to be perhaps handled better. As of now you can make use of the dashboard radio to hide the adminbar output invalid code to the browser, unless I'm missing something it takes literally seconds to effect a change as I mentioned.

P.S Apologies to all for not checking for duplicate tickets on the matter.

#3 @r-a-y
12 years ago

I would also recommend taking a look at #2024 before looking at this ticket.

#4 @hnla
12 years ago

  • Resolution set to duplicate
  • Status changed from reopened to closed

and I would agree :) and have done! what confuses me is what looks to be the eventual agreed solution proposed is tagged with 1.3 branch, a branch which isn't likely to be ready for some time. I guess there is a reason and that that reason escapes me. I will deal with this myself which isn't an issue and bow out and close this ticket as it doesn't serve any real purpose, other than to draw attention to an issue discussed and resolved? My parting comment would be not to overlook the importance of a correctly structured DOM and not fall into the trap of assuming all is well simply as the tag soup parser attempts to correct the error. Oh if only strict error handling was the case as in XHTML :(

#5 @TobiasBg
12 years ago

Thanks for agreeing to better have it removed :-)

I have previously tried to have this resolved by moving it to the (then current) 1.2.4 milestone, but apparently Andy didn't feel like that and moved it back to 1.3.

I'm not sure about his reasons, but I accept his actions.

#6 @hnla
12 years ago

  • Milestone changed from 1.2.5 to 1.2.6
  • Resolution duplicate deleted
  • Status changed from closed to reopened

I have deliberetly re-opened this under the 1.2.6 milestone despite my desire that 1.2.6 be closed and no more tickets raised.

Twice in the last couple of days this actual issue has arisen with members sites that I have looked at, hidden adminbar logged out view results in the most heinous mal-formed markup, and is unforgiveable, the solution? discussed at far too much length.

tickets:
#2024
and Ray's
#2153 (suggesting same solution as I do above - although removing function is better)

Both cover this issue and it should have been corrected, and I should have not sat back when I saw Andy had moved it to 1.3 as there is no sane reason I can see to dismiss BORKED code to a later milestone.

Pleeeese can we deal with this?

#7 @johnjamesjacoby
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [3279]) Fixes #2024, #2448 (branch). Also adds missing new lines. Props TobiasBg.

#8 @johnjamesjacoby
12 years ago

(In [3280]) Fixes #2024, #2448 (trunk). Also adds missing new lines. Props TobiasBg.

#9 @hnla
12 years ago

Thanks JJJ

Sorry to force the issue but it was necessary and a simple? fix ;)

and I'll accept on behalf of Ray and myself a little recognition for spotting issue and proposing semi solution albeit having missed the original ticket :)

Note: See TracTickets for help on using tickets.