Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
There were some <br /> and <p> tags in content_profile.module (called from hook_user, not in any theme function) that i have been repeatedly unsetting in my own module.
This patch eliminates them entirely.
I did not move them to a theme function because they don't appear to serve any functional purpose.
Comment | File | Size | Author |
---|---|---|---|
#11 | content_profile-421980-11.patch | 1.42 KB | xjm |
#8 | content_profile_421980-8.patch | 931 bytes | xjm |
20090402_content_profile.module.patch | 825 bytes | AaronBauman |
Comments
Comment #1
AaronBaumanoops - wrong status on that one.
Comment #2
fagoDoes the default template still work fine without those html tags? Any reviewers?
Comment #3
gilgabar CreditAttribution: gilgabar commentedPatch works perfectly for me.
Comment #4
xjmPatch works for me; thank you.
Comment #5
fagoI just gave it a test and without the
there is no spacing between the "add profile link" and the next heading. Also I dislike removing the parent "#content-profile-view" element as this might break existing CSS rules of people. Thus better move it into a separate theming function or similar.
Comment #6
AaronBaumanComment #7
gilgabar CreditAttribution: gilgabar commentedIt's really poor practice to use br tags for spacing. That should always be handled with CSS. It is easy to use CSS to replicate the spacing provided by the br tag. It is nearly impossible to remove the spacing created by the br tag with CSS.
It's probably reasonable to leave the #content-profile-view element, but it would be advisable to switch it from a p to a div to make theming easier (using a p unnecessarily inherits all the p declarations used in other parts of a design). And I don't remember offhand what gets put in there, so I'm not sure this is the case, but if any block elements are inside a p that is invalid html. So better all around just to make it a div.
Comment #8
xjmI'd agree with #7. This reroll of the patch replaces the
<p>
tag with with a<div>
rather than removing it completely.Comment #9
fagoI agree with #7 too, but still just removing the
tags does just remove the spacing as I noted in #5. Let's re-add those space with css.
Comment #10
xjmThis CSS should do the trick:
Edit: found the stylesheet. I'll roll a combined patch shortly.
Comment #11
xjmHere. Had to bump the bottom margin to 3em to get the same amt. of whitespace by default.
Comment #12
fagoah, fine. However I noted that doesn't work for the links (add or view) yet - in that case still the whitespace is missing.
Comment #13
xjmHmm, there's no markup printed around the add/edit links, nor even a class on the link we could use. So, that'd need to be added to the module's rendering and then styled with CSS. Maybe it would be better to replace the
<br />
with a div around content profile's area for each type and apply the bottom margin to that.Comment #14
mpotter CreditAttribution: mpotter commentedPatch works well for me. I completely agree that modules should not use
for spacing (that is what CSS is for), and replacing the
with
within
which isn't strictly allowed).
So +1 for patch.
Comment #15
atolborg CreditAttribution: atolborg commented+1 for this change.
Comment #16
Pls CreditAttribution: Pls commentedYes, I agree that using br tags is totally wrong. Looks like we need a re-roll on #11 with some CSS for links margin's (probably would be good to wrap all links to a div and apply margin for that div). Let's do this together. Any volunteers?