Scenario:
- image is unpublished
- image is attached to published node
For anon user (and, I assume, users without enough rights), no image shows on the node. This is correct.
For admin, image DOES show. This is misleading -- the admin doesn't see the node as other users do, and there's no indication of this difference. Best fix might be to mark the image in some way, or show something like "[unpublished image]" in its place.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | image_attach_unpublished_3.patch | 2.3 KB | joachim |
| #13 | image_attach_unpublished_2.patch | 1.74 KB | joachim |
| #11 | image_attach_teaser_unpublished.patch | 962 bytes | joachim |
| #6 | image_attach_unpublished.patch | 1.44 KB | joachim |
Comments
Comment #1
drewish commentedeverything is working correctly. uid=1 can see everything... you could get this functionality by using views though.
Comment #2
joachim commentedDon't you think uid=1 should get some sort of hint, the same way unpublished pages are pink?
Comment #3
joachim commentedI'm reopening this, as I think you've missed my point.
Uid can see everything. But, by design, unpublished nodes are marked so the site admin is informed that he is viewing unpublished content.
Hence, the same marking should apply to unpublished images attached to a published node.
Comment #4
joachim commentedBetter title.
Bump & move to 6.x.
Comment #5
drewish commentedplease roll a patch for 6.x ;)
Comment #6
joachim commentedDone :)
Wasn't sure whether to use the existing 'node-unpublished' class and therefore get the background colour from core or themes. But the padding is needed, and there's no way to grab a .node-unpublished that is only an image (as IE6 doesn't do multiple class selectors properly).
So I've gone with 'image-unpublished'.
Comment #7
drewish commentedhumm... it seems like it would be better to use the existing class so that it matches the theme... i'm not going to code to IE6 since this is only for uid=1 and hopefully admins would have the good sense to use a better browser.
Comment #8
joachim commentedROTFL.
Works out fine anyway, as I forgot to make the changes to teasers...
When I look at doing stuff with the theme layer I'll neaten this up a bit, but as you say, uid 1 should have better taste. If not, they'll just get some extra padding.
Comment #9
drewish commentedwent ahead and committed this to HEAD.
Comment #10
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #11
joachim commentedI forgot to add this class for teasers too...
Comment #12
sun- We should rename both instances of $published into $class, because that is what the variable contains.
- In addition, defining an empty variable is pretty ugly. Can we also move the "image-attach-<style>" part into the $class variable and just concatenate it into the resulting markup? (note: also in both places)
Comment #13
joachim commentedDone.
Comment #14
sunClose to completion. :)
- Inline if statements should always wrap the entire statement in braces to ensure the proper logic and also help in readability, ie. $foo = $bar . ($baz ? $bee : $boo);
- You changed the string concatenation to the new standards for $class (good), but not for $info on the same line (bad). :)
- Please add your first changelog entry. :)
Comment #15
joachim commentedI didn't touch $info because of the 'one patch, one change' rule...
Comment #16
sunSorry for being nitpicky.
The changelog entry should form a proper sentence, not necessarily equaling the issue title, and start with "Fixed" (for bugs), "Added" (for features), "Changed" (for tasks), or "Updated" (for changes that only 'react' to external conditions such as changes in Drupal core).
Exactly same changelog entry is used as commit message.
As mentioned in the handbook page, phrasing an entry can be a challenge. In this case, I'd propose something like
Comment #17
joachim commented"Fixed no css class" really isn't proper grammar... ;)
Comment #18
sunHere we go.
Comment #19
joachim commentedCommitted to both HEAD and 5-2 branches.