This patch implements alt attribute and title attribute support for images in the module.
You'll find it under the file formatter handling, Manage File Display. This should give maximum flexibility, as discussed with @tsvenson, @letharion and @fabsor. This was done under Media Initiative Sprint in Gothenburg (April 29th).
It uses [file:name] token as a default at the moment. If #1553020: File entity overrides existing [file:name] token is fixed/changed this will need a slight change but it won't break or explode, it will just change values.
Token support is implemented for these values so if you have the token module (for field tokens) and add fields for alt and title you can set them there and you'll have user-controlled alt and title attributes.
Comments
Comment #1
lawik CreditAttribution: lawik commentedForgot my screenshot.
Comment #2
lawik CreditAttribution: lawik commentedFixed patch naming.
Comment #3
attiks CreditAttribution: attiks commentedone 'the' too many
Comment #4
tsvenson CreditAttribution: tsvenson commentedWorth mentioning is that when #1292382: Make it possible to create any number of custom File Types lands and it is possible to create custom File Types, the plan is to add an out-of-the-box feature that includes fields for alt and title that will be possible to enable with a checkbox in the Edit tab.
It will also be smart enough to detect when an added file is of a mime "image/*" type and only make the alt attribute field available for those.
Comment #5
tsvenson CreditAttribution: tsvenson commentedSetting back to needs work due to posting at the same time as #3
Comment #6
fabsor CreditAttribution: fabsor commentedSome very small things:
This line should be wrapped so that it doesn't exceed 80 chars.
This comment should probably end with a period.
This comment should probably end with a period.
Comment #7
lawik CreditAttribution: lawik commentedFixed feedback. Thanks.
Comment #8
fabsor CreditAttribution: fabsor commentedThis works for image styles, but things break if you pick original image. You need to add the alt and title parameters to the theme_image call as well as theme_image_style.
Comment #9
lawik CreditAttribution: lawik commentedGood catch. Fixed.
Comment #10
tsvenson CreditAttribution: tsvenson commentedChange to: "The text to use as value for the img tag alt attribute. Can be left empty. If the contrib token module is available, field tokens can be used."
Change to: "The text to use as value for the img tag title attribute. Can be left empty. If the contrib token module is available, field tokens can be used."
Comment #11
lawik CreditAttribution: lawik commentedAdjusted the text from discussions.
Comment #12
fabsor CreditAttribution: fabsor commentedAlright, I see no other problems with this patch, all functionality seems to work well, and it's fairly easy to configure. RTBC as far as I am concerned.
Comment #13
adnasa CreditAttribution: adnasa commentedgood work ;)
Comment #14
Dave ReidThis can be cleaned up because we don't need to define variables here if we're just going to use them once. The array should also be fixed as per the coding standards to have each key/value on a new line, rather than wrapped at 80 characters.
This should use a conditional module_exists('token') to show the list of available tokens properly (http://drupal.org/node/1327676) - in other words follow core standards by not attempting to display the tokens ourselves and let token.module display the UI.
Comment #15
mgiffordComment #16
Everett Zufelt CreditAttribution: Everett Zufelt commentedI haven't read all comments, and am not sure I follow the summary. I will way that it is great to see improved accessibility, but that usingthe file name for alt / title is pretty much useless to improved accessibility. Can someone explain, or point me to the comment where it is explained, how useful values would be used for these properties?
Comment #17
Dave Reid@Everett: I think the idea is that this makes it easier to have a field on the various file types called 'Alt' and then be able to use a default of [file:field_alt] rather than [file:name]. Note that with File entity, the file name is actually a real user-changable text (i.e. 'My cool image') so it's more useful than the actual file name (i.e. 'myimage.png').
Comment #18
Everett Zufelt CreditAttribution: Everett Zufelt commentedCool, thanks :)
Comment #19
tsvenson CreditAttribution: tsvenson commented@Everett: If you install the contrib token module you will be able to use any field you create for the file type as source for the alt/title attributes. Reason we use the file name now is that it is the only editable field for a file available out-of-the-box. This gives you great flexibility.
Also see my #3 comment for info about the next phase of this, which can't be implemented until that other patch lands.
Comment #20
lawik CreditAttribution: lawik commentedGood feedback.
Ah, those variables. I somehow felt that they would come back and bite me. At the start we were working on some alternate defaults. They made sense then. They might return in the next step but for now they're gone :)
Since I'd argue we lose some usability for removing the homebrewed token table I added a note that the default value is [file:name] so you can see that you can use that even if you don't use the token module. It's a minor description text but I think it might help people.
Other than that. Something about the CSS on that page makes the token_tree style look like crap, with the arrows out on the edge. Haven't checked if it's a known issue yet. Including scrot.
Comment #21
tsvenson CreditAttribution: tsvenson commented@lawik: The issue with that the arrows are to far left for the tokens list is known and nothing to do with Media or your patch. Just FYI.
Comment #22
lawik CreditAttribution: lawik commentedMmm, figured as much.
Comment #23
RobW CreditAttribution: RobW commentedOK, sorry for the edit hiccup. Had to re-read #4 and #17 to make sure I understood what I was asking about.
Is the idea that the alt/title fields in #4 are going to be attached to the content/context the file entity is being displayed in, so that an image of a dog catching a frisbee could be
alt="a German Shepeard jumping three feet in the air"
on one page andalt="Rob and Poochie enjoying some exercise in the park"
on another? The problem with the theme-media-or-file-field-as-alt methods has been that tying those attributes to the source file breaks either reusability of files or any real accessibility.For reference, there's some similar conversation in a D8 core issue, #1291262: Add 'alt' and 'title' tokenized text options for image formatters, and a 'title' option for the generic file formatter, and in a media issue that's also patching its own file entity field for alt method, #1343022: Adding alt and title attributes from fields to image markup.
Testing this and it's working well.
Comment #24
lawik CreditAttribution: lawik commentedWell, this patch handles setting generalized title and alt for images. Mostly this would be the same wherever the image appears, but in some cases you might choose to have the image use the [file:name] token (not the filename on disk in file-entity, it's better ;) when displaying the image in a teaser but show the [file:field_title] when showing it in full context.
But if you want full control for every case where you are using the image that's a later issue concerning a way to override it. It's obviously not a problem in WYSIWYG where you can just set your own. But I believe the fix for overriding this would end up in the Media module as it handles the front end of making media files visible.
The discussion in media actually sparked this fix (see my comment at #45) as we saw something of a better way of solving it. Higher flexibility. Maybe I'll dive into the core discussion to seek wisdom later ;)
Comment #25
tsvenson CreditAttribution: tsvenson commented@RobW: When we where discussion the best way of implementing this during the sprint, being able to override values was mentioned too.
Thing is, its a far more complex issue that it initially look like. Not only do you want to be able to override, but in many cases you also want to prevent overriding completely and give full control to the uploader on what will be allowed when reusing a file.
So, we decided to come up with a solution that will use standard fields so that when the #1227706: Add a file entity access API patch lands it will work with this too.
Right now though, you have the possibility to set context unique alt/title for images you embed using WYSIWYG.
However, being able to override, and control what can be overridden, is out of scope for this patch. It is something that needs to be discussed in much more detail so we can come up with a plan that gives us a rock solid and flexible solution.
Thanks for the tip on the core issue though, didn't know about that one and will add a comment there so we avoid to competing solutions.
Comment #26
steinmb CreditAttribution: steinmb commentedDo we have an issue for this?
Comment #27
estepix CreditAttribution: estepix commentedHi, I'm not sure how this actually works. I updated to 7.x-2.x-dev, applied the patch on #20, enabled Image display in File Types > Image > manage file display, what else do I need to do? I have a few images uploaded via Media module but none of them get the alt and title markup added.
I'm sure that I am missing something, could anyone point me in the right direction?
Thanks in advanced
#Update: if I go to Content > Files > click an Image file, the image is displayed and the markup is shown correctly, however if I load the node where this image is attached there are no alt and title attributes.
Comment #28
tsvenson CreditAttribution: tsvenson commented@estepix: Out-of-the-box it populates the alt/title fields using the [file:name] token. You can then, see screenchot in #3, customize the texts.
If you also enable to contrib token module, you will be able to use field tokens. That mean that you can simply create your own alt/title fields to your files and then populate the attributes with those texts using those field tokens.
There will be a much better out-of-the-box solution though, but as I mentioned in #4 it can't be fully implemented until other feature as completed first.
Comment #29
Pomliane CreditAttribution: Pomliane commentedComment #30
chris_h CreditAttribution: chris_h commented@estepix I tripped on this. Make sure in managed display that the image field format is rendered file and set to the correct file type display
Comment #31
gmclelland CreditAttribution: gmclelland commentedPatch in #20 no longer applies against the dev versions of media and file_entity modules.
Comment #32
Pomliane CreditAttribution: Pomliane commented#20: file_entity-image_file_alt_and_title-1553094-20.patch queued for re-testing.
Comment #33
City-busz CreditAttribution: City-busz commentedI modified the following things in lawik's patch:
1. Do not print the token itself if its value is empty by adding 'clear' => 1 property in token_replace().
2. Do not set inital value to [file:name], leave it empty. I think that there is no real benefit by putting the filename into alt and title.
Comment #34
tsvenson CreditAttribution: tsvenson commented@City-busz:
Comment #35
redndahead CreditAttribution: redndahead commentedSo I'm not sure I agree that this should be handled in the file display formatter. I would understand if the formatter handles if it's displayed, but not sure it should handle the value of that content.
Here is how I would expect it to work.
Workflow for new files
Workflow for library
The only downside to this is you can't use tokens to specify the default values. I don't think that is such a large problem though for most people.
Comment #36
yatil CreditAttribution: yatil commentedI think that’s exactly how it should work. Although instead of disabling the output of an alternative text it should be possible to check of a “this image is purely presentational” checkbox. alt needs to be a required field unless that checkbox is checked.
And maybe it should lay out the advantages of good alternative text, like SEO reasons and of course accessibility. (But it would improve findability inside Media as well, I guess, so only advantages of a good implementation here.)
Comment #37
RobW CreditAttribution: RobW commentedPurely presentational images should be applied with CSS (usually as background images), so I don't think that checkbox is needed.
Comment #38
John Pitcairn CreditAttribution: John Pitcairn commented#37: while in theory that is true, in practice I often find my clients will require that some purely presentational images must also be printable without any modification of browser settings, which means they must be real images, not CSS backgrounds. In that case an empty alt attribute is essential.
Comment #39
mfer CreditAttribution: mfer commented@redndahead Your work flows seem sane. If the image file entity had those two default fields and they were use to pre-populate the media module UI that could be very useful. +1 to this flow.
The patch in #33 doesn't quite cut it. This should not be part of the formatter except to display the title/alt if they are set on the image file entity. This really is content and content creators (in my experience) want to manually set these.
Comment #40
tsvenson CreditAttribution: tsvenson commentedyatil: You are aware of that alt is a required <img> attribute? If not, see http://www.w3schools.com/tags/tag_img.asp.
Comment #41
BarisW CreditAttribution: BarisW commentedGiven that yatil is an accessibility expert, I assume he means that an ALT tag should be empty if it used solely for presentational reasons.
So either it should be
<img alt="Nice description" />
or<img alt="" />
but never just a random string (like filename) or left out.Comment #42
mgifford@BarisW - just adding a link to your point http://webaim.org/techniques/images/alt_text#null
Comment #43
redndahead CreditAttribution: redndahead commentedHere is a patch that adds alt and title fields do the image file type. This is useful for the patch that is in this issue: http://drupal.org/node/1307054#comment-6082716
Comment #44
redndahead CreditAttribution: redndahead commentedI created a video, no sound, of what the workflow looks like with these patches.
http://youtu.be/4kVcyY2soDo
Comment #45
RobW CreditAttribution: RobW commentedThanks for the link explaining the reasoning behind the empty alt attr. I still think adding a "presentational only" checkbox isn't a great way to go. At the least it will add confusing ui busyness for the majority of media users, and at the worst might give some users ideas and encourage bad behavior.
Maybe an optional helper module could add a presentational option, and use the aria
role="presentational"
instead of an empty alt. Pretty sure that's the current recommendation, and if it has good browser support would be a more forward looking way to take care of this.Comment #46
redndahead CreditAttribution: redndahead commentedSince the default at the moment is blank, I am keen to not add any additional handling. As long as someone doesn't enter anything in the field it will create an empty alt text.
Comment #47
fabsor CreditAttribution: fabsor commentedI don't think I like the idea to add an extra field to the image bundle. File entities will soon have completely configurable bundles. This means any bundle could possibly allow uploading of images. Should we add this field all the time every time a user creates a bundle that allows images? I think that could be left up to the user to decide.
Comment #48
redndahead CreditAttribution: redndahead commentedAlt and title attributes get added everytime to an image anyway might as well make it editable. One could use field permissions to hide them if they would like to.
Comment #49
mfer CreditAttribution: mfer commented@fabsor I think we need to think of practical examples and uses in addition to the building blocks. In this case our media handling system does images and the image component needs to handle that well. So, image file entities should handle alt and title text cleanly. Drupal is a CMS application and this is a common use that just needs to work out of the box to solve problems.
Now, we could do images on video file entities or audio entities. Practically speaking, I'm not sure what the best way to handle this is. Should the images on the video file type be image file entities? Should they just be fields? This is a more complex problem.
We can iterate and this need for images is quite important and immediate. We need it and can use it now. This is a practical solution to a practical problem.
Media handling in Drupal needs love and this is one way to do that.
@redndahead I hope to have time to look at the patch soon. The video in #44 shows exactly what I would like to see happen. Nice work.
Comment #50
redndahead CreditAttribution: redndahead commentedBy the way if you want the ability to set the alt and title text when you upload a new file you'll need to apply this patch also. #1426730: Automatically open the file edit modal after uploading a file
Comment #51
plachDidn't have the time to read the full thread yet hence I'm not sure whether this has been already debated, but I would like to point out that not using fields for Alt and Title will make translating them far more difficult and error prone. We'll likely need to use the Title module or a similar hackish technique to replace properties with regular fields.Sorry, I didn't look at the latest patch. The approach in #43 looks far more sensible to me. Hope to test it soon.
Comment #52
yatil CreditAttribution: yatil commentedComment #53
Letharion CreditAttribution: Letharion commentedSomewhat related #1639318: theme_image() should add a default but empty alt attribute.
Comment #54
Letharion CreditAttribution: Letharion commentedI definitely agree with the workflow as suggested in #35, but I disagree with the implementation, and I'll try to explain why the initial approach turned out the way it did. However, I will deliberately not comment on the library workflow part, as the file entity module exists largely to _separate_ the front end from the backend. Whatever solution we pick, should support different UI:s, so that contrib can add lots of awesome and sparkles on top.
When we came up with the first approach, we really wanted to avoid hardcoding fields into the module. Drupal "by default" comes with a "Page" and a "Article" content type, but they are not hardcoded into core. Instead, the Standard profile takes care of creating these content types and their fields on installation.
I believe we should take the same approach with alt and title tags. I suggest we stick with the token approach, and do _not_ provide a direct source of this information in this module, but rather have "Standard" create the source on installation, and insert the appropriate token as well. This way we will still have the fields as suggested in #35, and we can support the convenient workflow, yet still allow heavy customization, for example by different distributions; such as Commerce Kickstarter, NodeStream, and others.
A second benefit of the token approach is that it's very flexible. It's easy to string several values together for the title field for example, such as "[node:title] - [node:field_lead]", and using computed field, even more possibilities open up.
#33 does get it right though with removing the tokens by default from the patch. We added that because alt is required, however, the standard also states that it's better to have an empty alt, than a useless one.
The only remaining problem I see with the token approach, is that it is fairly odd. It's not a very common workflow in Drupal, especially not in Core. Then again, with the "Standard" setup suggested above, beginners should not need to face that problem.
Comment #55
redndahead CreditAttribution: redndahead commentedSo the problem with tokens is where to put the UI for it. Display formatters are extremely confusing and having to set up the alt and title tokens for each formatter is kind of ridiculous plus it really isn't formatting.
The only other place we can put it is with the fields. Maybe we can add an extra set of configurations on the fields page below the fields listing that let's you use tokens. Maybe this is more like an attributes section where you can specify any attribute and use tokens for them.
Comment #56
Letharion CreditAttribution: Letharion commentedHaving things in the field configuration does seem like a better approach, I agree. I'll see if I can re-roll that.
Comment #57
RobW CreditAttribution: RobW commentedComing here after a lively Media IRC discussion. Some important considerations about title:
Title does not add accessibility or useful information to:
and its use on images is actively recommended against by the W3C (in their HTML5 "Useful Text Alternates" draft). We should consider replacing the title attribute field with a generic caption or figcaption field usable in a default figure/caption formatter or custom theming. Let's stay ahead of the curve!
Sources:
http://dev.w3.org/html5/alt-techniques/#secm7
http://www.paciellogroup.com/blog/2010/11/using-the-html-title-attribute/
Comment #58
mrfelton CreditAttribution: mrfelton commentedSeems that patches #33 and #43 are both required... should they be rolled into to a single patch?
Comment #59
Pomliane CreditAttribution: Pomliane commentedWith #33 as is, token_replace() causes weird display of some characters (namely apostrophes) in alt/title. texts
Here is a single patch of both #33 and #43 including a fix ('sanitize' => 0) for this behaviour.
Comment #60
Pomliane CreditAttribution: Pomliane commentedComment #62
Pomliane CreditAttribution: Pomliane commentedLet's retry.
Comment #63
gmclelland CreditAttribution: gmclelland commented#62: Alt-and-Title-support-for-Images-1553094-62.patch queued for re-testing.
Comment #64
capellicAwesome. Thanks for the work!
Comment #65
sylus CreditAttribution: sylus commentedWorks for me :) Great job!
Comment #66
mrfelton CreditAttribution: mrfelton commentedAll good from here too. This is awesome.
Comment #67
mrfelton CreditAttribution: mrfelton commentedI take that back. The patch in #62 still doesn't include the update hook from #43.
Comment #68
Pomliane CreditAttribution: Pomliane commentedOops, yes, #43 was missing, sorry :/
Here is a rerolled patch.
Comment #69
jeffschulerI'm having some issues with this, (using the patch in #68 here and #1307054-40: Accessibility - Media browser image alt and title fields.)
I'm using Display Suite to lay out a node type that has image/Media fields, then colorbox as the display format for those images. I can set the alt/title values for the images (where they're attached to the node, as well as from the file/%/edit page,) -- and they're saved successfully -- but I don't see alt/title attributes when the node is displayed. I added some
dpm()
s tofile_entity_file_formatter_file_image_view()
and see them displayed on the node edit page, but not the node view page;file_entity_file_formatter_file_image_view()
is not being called. In fact,file_view_file()
is not being called, either, (it is on Edit, but not on View.)I tried disabling Display Suite and Colorbox entirely, but still,
file_entity_file_formatter_file_image_view()
is not being called and the alt/title attributes are missing.Also, when I do see the
dpm
output on the edit page,$file->field_file_image_alt_text['und'][0]
has a value, but$display['settings']['alt']
and$display['settings']['title']
are empty, and the alt/title tags on the images displayed in the form field are empty.Any ideas? What am I misunderstanding?
Comment #70
mrfelton CreditAttribution: mrfelton commented@jeffschuler - I think you need to use the Rendered File formatter to get the alt/title tag support. You also need to make sure that you have configured the alt/title fields (using tokens probably)for your file view mode, since they they are blank to start with. See attached for example config.
Comment #71
jeffschuler@mrfelton: thanks!
I hadn't realized, either, that there's a Media Colorbox module...
Comment #72
brunodboWorks great! Not sure what else is needed before this can be committed?
Comment #73
HyperGlide CreditAttribution: HyperGlide commentedhow do i apply this patch via drush to test?
Comment #74
mpgeek CreditAttribution: mpgeek commented@hyperglide, http://drupal.org/project/file_entity/git-instructions, associate the repo with your copy, and use the patch instructions.
Comment #75
HyperGlide CreditAttribution: HyperGlide commented@mpgeek -- Thanks. Got the patched applied. Testing will update with results shortly.
Comment #76
HyperGlide CreditAttribution: HyperGlide commentedHello my apologies if this post is a big long, I am really looking forward to this patch being committed and know lots of sites will depend on it.
To review the patch I looked at:
#35 Workflow for new files
#35 Workflow for library
#44 - http://youtu.be/4kVcyY2soDo video on workflow
----------------------------------------------------------------------------------------
For our application there are (2) different applications for where media is used and the "alt / title" path will assist us:
The review I have look at will test both applications for both new and existing files.
I have also installed patch -- #1426730: Automatically open the file edit modal after uploading a file
Notes for Core Image Field within a node -- Loading new media:
Notes for Core Image Field within a node -- Loading existing media from library:
Notes for Long Text / WYSIWYG Field within a node -- Loading new media:
Notes for Long Text / WYSIWYG Field within a node -- Loading existing media from library:
----------------------------------------------------------------------------------------
Observation:
Understand not related to this patch.
Understand not related to this patch.
I suspect this is not related to the patch.
Summary -- Notes:
I hope this helps and if any questions please let me know. Look forward to the replies and thoughts but most a solid RTBC patch.
Thanks for all the great efforts!
Comment #77
knalstaaf CreditAttribution: knalstaaf commentedAfter applying the patch of #68 it's possible to add a title or alt tag through the edit page of a node.
Problem is: it's not being displayed in the img-tag. The possibility to add an alt-tag is useless if it's not being displayed ofcourse. Did I miss something? I'm also missing the Manage File Display tab. (I also applied this patch for what it's worth).
Edit: I figured out that the Manage File Display tab can be found in Structure » File types » Image: Manage file display. Yet I don't see the option to set the alt- and title tag as shown in the screenshot.
Edit 2: Now I have the alt- and title-fields as shown in the screenshot. I took me to apply the very first patch from the starting post of this thread. See, this is confusing. I thought the patch of #68 was actually the final version of
0001-Implemented-Alt-attribute-and-Title-attribute-for-im.patch
.I'm still stuck with this display now, and I have no idea how to fix it:
Edit 3: I fixed the problem of "edit 2". I had to go to "admin/structure/file-types/manage/image/display" and set the alt and title text to "hidden".
I've been so totally lost and confused in the patches and overall communication (spread over multiple threads) about this issue concerning the alt-tags for the Media module. It's horrendous frankly, I lost loads of time.
Maybe this can be avoided in the future by placing the necessary patches (or links to it) at the top of the thread, be it by the thread starter manually, or by the Drupal.org site automatically.
Comment #78
HyperGlide CreditAttribution: HyperGlide commented@knalstaaf -- can you list all the patches you used?
I used:
edit -- change 64 to 68.
Comment #79
knalstaaf CreditAttribution: knalstaaf commentedThese are the patches I used to achieve the result I have now:
I didn't need to apply the patch that would open the file edit modal after uploading to get this far (so I guess that's ok).
Comment #80
HyperGlide CreditAttribution: HyperGlide commentedThanks for the reply -- what is 1426730-d7-6.patch required?
I applied the final patch from that thread -- was this feature not included in the patch?
Comment #81
knalstaaf CreditAttribution: knalstaaf commentedAfter reading the whole thread through it must be included indeed. I didn't apply the final patch from that thread, and everything works ok. But maybe I should nonetheless just to make sure.
Comment #82
HyperGlide CreditAttribution: HyperGlide commented@Pomliane -- can you please confirm any dependencies for your patch #68?
Comment #83
tsvenson CreditAttribution: tsvenson commentedI have now reviewed the patch in #68 and it applies clean on the current dev version.
Great work so far everybody. Looks like we are close to getting a smooth and flexible alt/title support in soon.
For me it worked as expected, the fields where created after running updatedb and after adding the tokens the alt and title text where properly added to the markup both for the media field and when embedding using wysiwyg (tested on CKEditor just).
The only remark I have is that the tokens for the supplied alt/title fields should populate the admin/structure/file-types/manage/image/file-display image settings on the "Manage File Display" tab too and not have to be done manually.
Comment #84
HyperGlide CreditAttribution: HyperGlide commented@Pomliane -- not certain if have had a chance to review the thread but do hope we can test another updated patch soon.
Comment #85
knalstaaf CreditAttribution: knalstaaf commentedI had another project with the same demands and this time things went faster.
To whom who may be dwelling with the same case, these are the steps I took (to avoid the trouble I met in #77):
This enables the alt and title fields as described above. Works brilliantly (thanks for that).
Here's the tricky part:
[file:field_file_image_alt_text]
and[file:field_file_image_title_text]
In order to use the image styles I configured, I had to apply this patch. This patch let's you override the default styles of the "Rendered file" in a Views field.
When this patch was applied I clicked the Image field within my View, which let's me now output that image with the formatter "Rendered file", view mode "Default" (or "Standard"), and override the image style with "thumbs_news".
Now the images are in the right image style, and they have the necessary alt and title attributes.
Thank you for the efforts to make all of this work.
Comment #86
HyperGlide CreditAttribution: HyperGlide commented@knalstaaf can you comment please if you feel this patch is RTBC or what work is required?
Thank you
Comment #87
knalstaaf CreditAttribution: knalstaaf commentedSure, I think it's RTBC. I'm even quite enthousiastic about it :)
Comment #88
agoradesign CreditAttribution: agoradesign commentedI agree. I've also used this patch in two or three projects now and haven't seen any problems so far :)
Comment #89
HyperGlide CreditAttribution: HyperGlide commentedChanging status to RTBC
Please note -- #83
Do not believe this was completed or required for this to be committed.
Comment #90
tsvenson CreditAttribution: tsvenson commentedWhile the patch technically does its job and doesn't seem to contain any bugs, my concern in #83 is a usability/UX issue and still unsolved.
Default should be that the alt/title tokens are populating the configuration and thus outputted without requiring the user to manually configure it.
This is a sitebuilder usability/UX issue and we need to be better on fixing those.
Comment #91
mgiffordThis thread has been going on since April. There's code that makes an improvement that everyone agrees on. Let's get that into git & open a new issue to deal with the token suggestion proposed in #83 & clarified in #90. For that reason I'm bringing this back to RTBC and ask that @tsvenson flesh out his proposition a bit more in a new issue.
From an accessibility perspective (and that is why alt tags are being presented here), I would want to make very sure that the pre-populated text is actually providing useful content. Images with alt text like:
are useless. The alt text needs to be specific of the image and also be related to the context that it is being placed in. It's a matter of describing the image and having a sense of what it is doing for visual users. They don't have to be that detailed to be useful, but that is the goal.
Here's the recommendations from the W3C about how to deal with alt tags:
http://dev.w3.org/html5/alt-techniques/#recommendations
But ya, let's discuss this in a new issue.
Comment #92
jherencia CreditAttribution: jherencia commentedI agree #91, most users do not know how to apply the patch and it's not even clear which one to apply. This important enough to get in as soon as possible.
Comment #93
tsvenson CreditAttribution: tsvenson commentedAs suggested in #91 I have now created #1792334: Prepopulate alt/title attribute configuration with the default field tokens as a follow up issue.
Lets get this one in before unstable7/alpha1/whatever...
Comment #94
HyperGlide CreditAttribution: HyperGlide commented@tsvenson -- thank you for setting up the new issue got the first follow.
Comment #95
jamesbenison CreditAttribution: jamesbenison commentedHey,
I wish I had more time to help with this. But for now this is the best I can do...and you may not like it.
Every time an image field is created in drupal it has table columns for height, width, alt, and title. This is by default. It is in core. It happens whether you like it or not (yes you have to pick title, but that's a tangent).
Somehow this module finds it necessary to create a new table for image dimensions and new fields for the other attributes.
It's positively bass ackwards. Those table columns are already sitting there waiting to get filled in. And you know what...if the default attribute columns store the data then...VOILA!...suddenly it works with EVERY module that uses image fields because it's done just like core!
The dimension attributes get filled in automatically. So that's a redundancy that I don't understand how it justifies its existence. It has a separate table because...?
But wait...it gets better! Using fields for alt and title does NOT create any added benefit. In fact, it takes away flexibility. First of all, are people planning to put a wysiwyg editor on it or something? And using the fields approach means that the alt and title fields get created once...and only once...and you are stuck with them and their settings for the attributes of every image in the media module. For instance, try to create alt fields with different character length limits using core image and using media module image with the patch above. Report what you find.
Comment #96
jwilson3Based on the content in #95, and the fact that there is plenty of useful information, spread across the 95 comments here and in the other issues that were all marked as duplicates of this one, I'd say its time this ticket got an issue summary explaining what the issue is, and why the architectural decisions were made to go the way they've gone thus far.
Comment #97
jwilson3^ should have been "needs work".
Comment #98
HyperGlide CreditAttribution: HyperGlide commented@jamesbenison how does your remarks relate to the what that had been done here -- #1343022: Adding alt and title attributes from fields to image markup
There was a discussion about using alt - title text in all media types within media. This maybe one reason why a new table was created vs. using that from core.
@Dave Reid as the maintainer can you please offer your remarks on how to move this toward RTBC with the code that best suites the module and community needs.
Hoping we can keep the discussion going so there is an agreed way to code this and commit it.
Comment #99
HyperGlide CreditAttribution: HyperGlide commentedHad a long and good discussion about this patch and other related work -- if you like you can read over the irc log -- http://pastebin.com/V90ffqXA
Will post a summary of main ideas shortly.
Thanks to RobW and tsvenson.
Comment #100
HyperGlide CreditAttribution: HyperGlide commentedAs noted in #99 there was a good discussion in IRC related to the issue of #1553094: Alt and Title support for Images.
Related to #95 the proposed use of the core tables does not fit based ont he following:
Other related issues that should not delay this path:
Related issue no. 1 -- #1792334: Prepopulate alt/title attribute configuration with the default field tokens
There still seems to be confusion over this path, to confirm:
This issue just helps to ensure that media is properly configured for suite builders.
Related issue no. 2 -- Re-use / Multiple Use of media each having the ability for a unique alt.
The current patches in this issue only permits for (1) alt tag that used in all instances for that piece of media.
A single alt ruins search/seo and accessability in this situation.
Use example:
Recommendation is to commit the patches as shown in:
Comment #101
RobW CreditAttribution: RobW commentedGreat summary of the issues, HyperGlide.
I don't think #1244476: Add an "Override image style" setting to the rendered file formatter needs to be committed to address the alt problem. The two possibilities I see for storing per-context alts are either binding contexts to specific revisions of the alt field (which IMO is hacky), or adding db columns to the file field referencing the file entity to store alt and other per-context attributes. All of this should be discussed in a follow-up though, as this issue is a big ol' RBTC.
Comment #102
ParisLiakos CreditAttribution: ParisLiakos commented#68: Alt-and-Title-support-for-Images-1553094-68.patch queued for re-testing.
Comment #103
ParisLiakos CreditAttribution: ParisLiakos commented#68: Alt-and-Title-support-for-Images-1553094-68.patch queued for re-testing.
Comment #105
mgiffordIn a review I noticed in #91 I seem to have removed some tags. Not sure why, so adding them back in. I'll assume some tech glitch of some sort.
Comment #106
mrfelton CreditAttribution: mrfelton commentedPatch from #68 no longer applied cleanly. Updated version attached. Lets try those tests again...
Comment #108
mrfelton CreditAttribution: mrfelton commentedRenamed update hook so we dont have duplicate function.
Comment #109
mrfelton CreditAttribution: mrfelton commentedSetting back to RTBC, since nothing has changed since #100 where this was marked as RTBC.
Comment #110
jwilson3I compared the patch in #108 to #68 to confirm this as a valid reroll. RTBC++
Comment #111
ParisLiakos CreditAttribution: ParisLiakos commentedThose need to be updated to match new commits. See #1296268: Add Preview and Teaser view modes and #1051090: Revamp file view modes: migrate media_small to teaser, media_large to full, media_preview to preview; deprecate link & original
Comment #112
HyperGlide CreditAttribution: HyperGlide commentedany taker? or guidance on what is required?
Comment #113
ParisLiakos CreditAttribution: ParisLiakos commentedChange:
media_small -> teaser,
media_large -> full,
media_preview -> preview
only apply on link & original if variable
media__show_deprecated_view_modes
isTRUE
if i find some time i will do it but its not something hard
Comment #114
RobW CreditAttribution: RobW commentedThe code rootatwc pasted defines the formatter settings for the alt text field on the file entity. From what I understand it doesn't have any effect on the actual printing of the alt text in the alt attribute itself. IMO, we should change default to be hidden as well, since the expected behavior is that the alt is in the image, not printed underneath it. Any changes need to be done for title, too.
I would take care of this but my laptop is in the shop for two more days having its screen replaced. Pity me.
Comment #115
paranojik CreditAttribution: paranojik commentedWell, I hope I got this right...
Comment #116
RobW CreditAttribution: RobW commented@paranojak: "small" should be "teaser". Aside from that, looks good.
Comment #117
HyperGlide CreditAttribution: HyperGlide commentedreplaced "small" with "teaser".
Comment #118
ParisLiakos CreditAttribution: ParisLiakos commentedI think this is ready and #114 is addressed
landing soon
Comment #119
ParisLiakos CreditAttribution: ParisLiakos commentedHmm i think its time to commit this, although after reading and re-reading the comments here, i am not convinced about everything especially what is in #95.
Hmm i dont see how file_entity goes in core with 6 weeks remaining till feature freeze and us not come up yet with a file_entity acces API yet: #1227706: Add a file entity access API
I am sorry but i cant see it happening
I attached a patch with some code formatting and description changes, most important, taking the token browser to a dialog instead of showing it in the page and leaving for Dave Reid to review and commit
Comment #121
ParisLiakos CreditAttribution: ParisLiakos commentedyeah and bot decided to start misbehaving now -.-
w/e essentially its the same patch.
Comment #122
HyperGlide CreditAttribution: HyperGlide commented#119: 1553094-alt_and_title_support_for_images-119.patch queued for re-testing.
Comment #123
supradhan CreditAttribution: supradhan commentedI used 'Alt-and-Title-support-for-Images-1553094-117.patch' patch. Now I can see field to put 'Title' and 'Alt' tag and I can also change the file name. But none of them are applied when I save them and view (via firebug I cannot see any title, alt tag and file name unchanged). However when I edit, there is 'alt text', 'title text' and 'renamed file name'.
Comment #124
ParisLiakos CreditAttribution: ParisLiakos commentedYou need to set the token, under
Structure => File types => Manage file display
Comment #125
HyperGlide CreditAttribution: HyperGlide commented@ClsSharp -- did you follow the steps in -- http://drupal.org/node/1553094#comment-6496832
Once you load the patch you have more to do -- this is a site builder issue as noted in the thread and issue -- #1792334: Prepopulate alt/title attribute configuration with the default field tokens
I would also use path http://drupal.org/files/1553094-alt_and_title_support_for_images-119.patch
Comment #126
HyperGlide CreditAttribution: HyperGlide commented@rootatwc :-)
Comment #127
HyperGlide CreditAttribution: HyperGlide commentedAfter applying the patch do not forget to run update.php.
Comment #128
tsvenson CreditAttribution: tsvenson commentedRe #123:
This is what I was afraid would happen due to that this patch wont work out-of-the-box until the follow up issues #1792334: Prepopulate alt/title attribute configuration with the default field tokens also lands. It will for sure cause a lot of confusion for users wondering what is not going on.
But, there is also another problem with this and that is that those two tokens are field tokens. As we know field tokens are not supported by core. They require either the contrib Token module or the Entity Token submodule that comes with the Entity API module.
Due to that there are known conflicts when both those modules are installed, we should not add a dependency in the .info file for any of them now. Some big modules requires one of them and thus often must avoid the other one. Also, there is the issue of that they doesn't agree on the use of "-" and "_" in the field tokens.
@HyperGlide is currently performing tests comparing how this works with the Token and Entity Token modules and will comment here with the result.
Comment #129
supradhan CreditAttribution: supradhan commentedThank you for help.
I uninstall modules. I used new patch(good UI) and installed modules again. I followed the steps http://drupal.org/node/1553094#comment-6496832. Installed Token, Entity API modules. I hid 'Alt Text' and 'Title Text' from 'Manage Display' and used token for them in 'Manage File Display' tab. I used medium image style. I created new field of type image with media selector widget. I used medium image style in display. But still there is no title/alt in the image. Am I missing something?
Comment #130
supradhan CreditAttribution: supradhan commentedAlso run update.php (msg: no pending updates) and no luck yet.
Comment #131
dddave CreditAttribution: dddave commented++++1 for not creating a dependency on Entitiy tokens. tsvenson is 100% right with his remarks.
(sidenote: it is a shame that this conflict still exists and now we are seeing negative repercussions due to this nonsense.)
Comment #132
supradhan CreditAttribution: supradhan commentedI am very confused. I am not getting any alt, title text in my node (Relative path: /node/8) where I do have the image field but I am getting alt/title text in the actual image (Relative path: /file/14). Is this the right behavior?
Comment #133
supradhan CreditAttribution: supradhan commentedGot it worked: Changed the display to 'Rendered File' and it worked.
Thank you all for helping.
Comment #134
PI_Ron CreditAttribution: PI_Ron commentedThat was a fair read. Thanks to everyone's work on this problem, it's appreciated by site builders :)
Comment #135
HyperGlide CreditAttribution: HyperGlide commentedFollow up to #128 from @tsvenson.
[file:field-file-image-alt-text]
or[file:field_file_image_alt_text]
.image
display mode is not enabled on/admin/structure/file-types/manage/image/file-display
.A separate observation for the recent tests showed that the
alt
andtitle
tages where displayed only when the content type's display mode (i.e./admin/structure/types/manage/article/display
) is set torendered
format. Either I have a configuration error or further documentation is required to make this patch easier for the 'average' site builder to implement.As much as I wish to see this committed, I am going to move back to "needs review".
Comment #136
tsvenson CreditAttribution: tsvenson commentedThanks for the great testing work @HyperGlide.
I think its fair to say that if we have this much confusion going on just working on this patch, then we should not expect less confusion from those enabling the module and expecting this to work.
We need to:
My suggestion is that by default we use the format for the contrib token as I believe it is the most used one.
Comment #137
jwilson3I think Token module should be the default baseline and it should have precedence; if entity token also exists, the user should be able to change it by hand, assuming they know why they would want or need to do such a thing.
Are there cases when an entity token might exist for an image attached to an entity, but not a regular token? Or can we assume that where there is an entity token, there is also always a regular token (assuming both modules are installed).
Comment #138
tsvenson CreditAttribution: tsvenson commented@jwilson3 as pointed out in #128 there are known cases when contrib Token and Entity Token conflicts and where it might be needed to pick one of them. Depending on what kind of site it is, that will then decide which one that has to be disabled.
Comment #139
HyperGlide CreditAttribution: HyperGlide commentedUpon discussion in IRC with @rootatwc and @tsvenson;
In reference to #135 the following actions require coding support.
With regards to the token concern:
(module_exists('token')) { //add token types of token module as default} elseif (module_exists('entity_token')) { //add entity_tokens token types}
admin/structure/file-types/manage/image/file-display
with the correct tokes. (i.e.[file:field-file-image-alt-text]
or[file:field_file_image_alt_text]
.Any volunteers to help implement these issues so we can retest the patch?
With regards to the
alt
andtitle
tags displaying only when the content type's display mode is set to rendered format concern rendered files works just fine. However the configuration is a bit different then how things work in core. Here are the basic steps required./admin/config/system/entity-view-modes
.FILE
group and selectAdd new view mode
.Label
./admin/structure/file-types/manage/image/display
and open upat the bottom.
Save
..
alt
andtitle
are set tohidden
./admin/structure/file-types/manage/image/file-display
and here is where the customization begins.Enabled displays
put a check inimage
.Display settings
and now select your desiredImage style
.Alt attribute
hase the required tokens. It should be in the format of either[file:field-file-image-alt-text]
or[file:field_file_image_alt_text]
. Depending upon which token module you are using.Title attribute
hase the required tokens. It should be in the format of either[file:field_file_image_title_text]
or[file:field-file-image-title-text]
. Depending upon which token module you are using.Save configuration
.Display settings
./admin/structure/types/manage/article/display
remember thatarticle
is only an example, will vary based on your content types name.FORMAT
toRendered file
.View Mode
.Update
.Save
.Thank you,
Hg
Comment #140
BerdirI'm a bit confused. This seems to work well, the only thing is that you manually need to add those tokens. And there's even a browser to select them from. And there is a follow-up issue to implement this: #1792334: Prepopulate alt/title attribute configuration with the default field tokens.
It's also not as complicated as described in #139, I don't really see why you need a custom view mode for this, you can just use the default one which makes most of these steps unecessary.
So why is this needs work because it's not done this automatically?
Edit: Fixed issue link.
Comment #141
HyperGlide CreditAttribution: HyperGlide commented@Berdir there is a feature request that the tokens are pre populated.
The token follow up issue is #1792334: Prepopulate alt/title attribute configuration with the default field tokens.
If you have alternate steps that do not required the custom view mode please provide them. If you only use the default view mode then you will only get 1 image format. Each custom view mode is meant to support each image style as defined in
/admin/config/media/image-styles
.If you feel it is RTBC please revert and assign to Dave Reid as he will be the one to commit.
Comment #142
RobW CreditAttribution: RobW commentedFrom what I understand it's needs work for integration with the two token modules. And maybe documentation, but I don't think that's a commit blocker.
Comment #143
HyperGlide CreditAttribution: HyperGlide commentedComment #144
HyperGlide CreditAttribution: HyperGlide commented@RobW -- can you commit it?
Comment #145
BerdirYes, that's the issue I meant, sorry for the wrong link, fixed that.
What I meant is that it looks to me like this issue is blocked on the exact thing that the follow-up issue is supposed to implement. That's a logical error :) Having the follow-up means that we don't need to fix it here otherwise the follow-up is not necessary. Feel free to correct me if I'm missing something.
Makes perfect sense to me to commit the generic ability to do this without having to use custom code and then improve the usability. I would have even been happy to just have the token_replace() support in there as a first step, everything else can be done manually, if required.
Regarding view modes. Yes, every view mode that you use needs to be configured to use the tokens for the alt and title. Your instructions make it sound like you need additional modules and have to use custom view modes to use this functionality but that's not the case, it works just fine when you configure it in the default file display view mode. You use multiple view modes because you need different image formats, not because this feature requires you to do so :)
Comment #146
RobW CreditAttribution: RobW commentedAside to Hyperglide's tutorial: I wouldn't create new view modes named for your image style. That's copying the deprecated Styles module workflow, and it sort of works, but a better practice is to use the view mode to describe the context of the content in the document (teaser, blog_feed_teaser, product_view, etc.) and then apply an image style (which can be named for its dimensions or its purpose, depending on whether you'd rather revise image styles or image formatters when you change your display) in that view mode. Otherwise you're forcing an artificial one to one application of view modes to image styles, which completely breaks down when you go to configure an audio file, or when you're using something like the
<picture>
formatter. Other than that, great step by step tutorial.@hyperglide: I'm not a file entity maintainer :).
Comment #147
ParisLiakos CreditAttribution: ParisLiakos commentedI actually like this...how about committing only the token replace support to the file display settings and leave fields out for now?:)
Comment #148
HyperGlide CreditAttribution: HyperGlide commented@Berdir
per
Correct it is not because the patch requires it, but rather I am not aware of any sites that use only 1 view modes. ;-)
Once this is committed. I would be more than happy to create a handbook page.
Comment #149
ParisLiakos CreditAttribution: ParisLiakos commentedpatch per 147
This way file_entity does not enforce any additional field on install, though if you want to add extra fields for alt and title you are free to do so and then assign those tokens to the file display settings.
what do you think?
Comment #151
ParisLiakos CreditAttribution: ParisLiakos commentedoops
Comment #152
HyperGlide CreditAttribution: HyperGlide commented@Robw
When you say:
Are you talking about the same workflow -- but using different nomenclature for the names? As the file display mode must be set to "rendered" this was the only way I could get the patch to work. If there is a different workflow can you please share some steps or jump on IRC. I would love to make the process cleaner.
Thanks!
hg
Comment #153
Berdir@rootatwc: If we can get the patch with the fields in, then that's perfectly fine with me. What I meant is that if we *couldn't* or it would be somehow blocked (e.g. discussions about the names or how they should work by default), than would be a viable alternative.
If there would be an easy way to support this, then it would be nice to have it optional, similar to the default minimal/standard install profiles (if it were in core, for example, then we could add the fields to the standard profile) But I will use those default fields too, I'm not against them in any way. Sorry if I was unclear.
Comment #154
RobW CreditAttribution: RobW commentedJust the naming scheme. Using "semantic" view mode names makes things easier after the setup process, sort of the equivalent of using a class called
.postlist-title
instead of.h2-240px
when creating css architecture.Comment #155
RobW CreditAttribution: RobW commentedEh, whoops.
Comment #156
HyperGlide CreditAttribution: HyperGlide commentedI would vote to put the fields in with the patch as the goal of the patch is to provide ALT and TITLE support.
If the concern is that the patch generates new tables then perhaps review the location the data is stored, however that does not change the need that the ALT and TITLE data. At the end of the day the data needs to go someplace.
Comment #157
BerdirAgain, The patch in #119 is RTBC. The patch #151 would only be necessary if someone would be opposed to add those default fields, and nobody seems to be.
Comment #158
ParisLiakos CreditAttribution: ParisLiakos commented#119: 1553094-alt_and_title_support_for_images-119.patch queued for re-testing.
Comment #159
ParisLiakos CreditAttribution: ParisLiakos commented119 needs a reroll
Comment #160
supradhan CreditAttribution: supradhan commentedIt is working with local images. But it is not working with Flickr images.
Comment #161
supradhan CreditAttribution: supradhan commentedAlso as we need to use 'Rendered file' option we cannot use shadowbox, lightbox modules at the same time.
Comment #162
mgiffordCan we treat Flickr images as a separate issue? If we can resolve this locally it will address 90% of the problem. Flickr can be dealt with after that.
Comment #163
HyperGlide CreditAttribution: HyperGlide commentedMgifford++.
Lets get this issue solved for local files first.
Comment #164
lokz CreditAttribution: lokz commentedWhen can we expect this "patch" to be included in module? Is it in DEV version already?
This is a "must have" functionality for photos on website (especially regarding SEO aspects).
Not related to this closely, but I want to point out that user should be able do add media gallery as field to another content type. I wrote about it in this request, but just want developers to know about this idea. Thanks:
Possibility for gallery to be added to other content type is needed
Comment #165
lokz CreditAttribution: lokz commentedComment #166
kingfisher64 CreditAttribution: kingfisher64 commentedApplying the patch alone is not enough as tsvenson in #128. Aside from the patch being committed what else needs to be done to get this "feature" working?
I for one would appreciate a post with some clear instructions on the whole process rather than going to external threads to try and get this fixed.
One place with all patches and clear instructions to the files they are patching in a bulleted list would be helpful :)
Comment #167
HyperGlide CreditAttribution: HyperGlide commentedPlease see the detailed directions in #139.
Comment #168
HyperGlide CreditAttribution: HyperGlide commentedPlease see the detailed directions in #139.
Comment #169
knalstaaf CreditAttribution: knalstaaf commentedI'm not sure if this is still of any importance, but I want to pop it in just in case: after applying patch #108 I got a WSOD (working on a local machine - php.ini times and sizes set to a reasonable maximum).
My first reaction was to
update.php
, which brought up this error:Fatal error: Cannot redeclare file_entity_update_7203() (previously declared in /Applications/MAMP/htdocs/my_web/sites/all/modules/file_entity/file_entity.install:537) in /Applications/MAMP/htdocs/my_web/sites/all/modules/file_entity/file_entity.install on line 553
EDIT: Do not follow my instructions below as they are (real) bad practice (see #171).
Comment #170
HyperGlide CreditAttribution: HyperGlide commentedPatch #119 is the latest -- but requires a re-roll.
Comment #171
agoradesign CreditAttribution: agoradesign commented@knalstaaf
Don't worry, it's not bad practice - it's superbad! ;) No really, that's not good. All The file_entity_update_xxxx() functions you see in the install files are implementations of hook_update_N. These are update scripts that modify the database, required by the module itself. The patch you tried to use, was already too old for the current development version, as in the mean time obviously a new feature was implemented that required a database update. The patch in this thread also needs a database update. At the time of writing it got the number 7203, but as time passed by and it's still uncommitted, the other update now has got update number 7203.
That's the reason, why some people already wrote, that this patch needs to be rerolled, that means corrected to work with the current development version.
A quick help for you: as you have commented the "real" 7203 update and have already run the "wrong" 7203 belonging to the patch of this thread, I would recommend to uncomment the "real" 7203 and rename it to 7204. And the run update.php again. Now both update scripts were executed. The order is historically not correct, but doesn't matter. And you module's version is officially 7204, as it would be, if you'd done it correctly from the beginning on...
PS: why did you choose the patch from #108 and not the one from #119? #119 also needs to be rerolled, but does have some (important?) changes/Improvements in comparison to 108
Comment #172
knalstaaf CreditAttribution: knalstaaf commentedThanks, agoradesign! Followed your instructions and all runs safe'n sound.
The reason why I picked #108: in the mammal world red stands for "danger". I saw that dreadful colour in #119 and an "I'm alright, eat me"-colour in #108. Basicly I thought #119 was the "dev"/unstable version of 108, hence the choice.
The fact that I'm also not familiar with the term "to re-roll a patch" might have pushed me to the things I did as well. Does "re-roll a patch" imply that you have to delete your previously patched module folder, add the latest clean (dev) version again to finally patch it with the latest patch?
I've been looking it up, but I can't seem to find the exact definition.
Comment #173
HyperGlide CreditAttribution: HyperGlide commented@Knalstaaf - The term role means that a patch need to be updated to be applied to an updated dev version of the code.
Borrowing from -- http://drupal.org/patch/reroll
So to fix it the patch has to be rebuilt or re rolled against the latest DEV version so it can be applied cleanly.
Comment #174
agoradesign CreditAttribution: agoradesign commented@knalstaaf:
in addition to the HyperGlide's description: you were right, that at the time of creation, a patch always applies to the latest dev version. And if you already are using a patched dev version, you're right that you can't use a patch twice and you'd have to run the new one against a clean (dev) version.
But the most important thing is, that you have to really take care if you use a patch that includes a db update script. As soon as you update the module the next time (no matter if it is a dev, alpha, beta, rc, stable,... version), you have to check, if your patched hook_update_N is really the same in the updated module. Otherwise you have to take care manually that all db update scripts are applied correctly
Comment #175
BerdirAttaching a re-roll. Also added support for the default tokens for those alt and title settings through hook_field_formatter_info().
Comment #177
Berdir#175: 1553094-alt_and_title_support_for_images-175.patch queued for re-testing.
Comment #178
GaëlGWorks for me.
Comment #179
HyperGlide CreditAttribution: HyperGlide commented@GaëlG -- Thanks for the quick test! Per#128 and #135. Did you test with both contrib Token module and Entity Token submodule that comes with the Entity API module?
Comment #180
GaëlGWell, it works but may not fit my needs. If I'm right, it needs "Rendered file" as a field formatter. Am I missing something or this implies the image can't link to the content page ?
Comment #181
HyperGlide CreditAttribution: HyperGlide commented@ GaëlG -- pls read and try -- http://drupal.org/node/1553094#comment-6638474
Comment #182
GaëlGI did it. I'm only asking if there's a way to output my image field with alt and title as a link to the node. Because this is possible if the field formatter is set to "Image", but AFAIK impossible if it's "Rendered File", which has to be set according to point 24 of #139.
How to have both title (or alt) and link to content?
Comment #183
Berdir@GaelG: Not with this patch, but it would be possible to add another formatter that would support to directly display an image and using the correct alt/title values. I suggest we look into that as a follow-up.
Comment #184
gmclelland CreditAttribution: gmclelland commentedGaëlG - Haven't tried, but perhaps http://drupal.org/project/file_entity_link would work or maybe this can be done with http://drupal.org/project/custom_formatters?
Comment #185
agoradesign CreditAttribution: agoradesign commented@GaëlG:
I'm also using Media module in our projects. To be forced using "rendered file" also disturbed me. Therefore I've implemented a small theme function, that I use in all my projects:
This enables the use of the good old image formatter for file entities with the new alt and title fields. It just sets their value to the item's alt / title values and then calls the default theming function.
Overall this is a quick and not too dirty solution for this problem :)
Comment #186
jonraedeke CreditAttribution: jonraedeke commentedThanks Berdir! - #175 Works for me too. I used the Token module to insert the values and the standard "Rendered File" format.
Comment #187
HyperGlide CreditAttribution: HyperGlide commented#jonraedeke per #175 the tokens should have been there automatically. Should not have required any intervention. Was the case for you? As noted in #175 the patch should best tested with both contrib Token module and Entity Token submodule that comes with the Entity API module?
Comment #188
jonraedeke CreditAttribution: jonraedeke commentedTried #175 with Token and Entity Token both enabled. Works fine. But if I don't put a token in the default fields in Manage File Display, I get empty alt and title output in Firebug.
Comment #189
HyperGlide CreditAttribution: HyperGlide commentedTested #175.
/admin/structure/file-types/manage/image/file-display
Nice work all!
Berdir++
Comment #190
GaëlG@gmclelland You're right, File Entity Link is the answer. Thank you so much.
Comment #191
knalstaaf CreditAttribution: knalstaaf commentedHello again, I used patch #175 on a brand new 7.x-2.x-dev (as an update), but it seems that I lost everything for some reason. When I go to "File Types" all is gone there... I also had this update-error:
Failed: PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'sitename1.file_type' doesn't exist: INSERT INTO {file_type} (type, label, description) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => [:db_insert_placeholder_2] => ) in file_type_save() (regel 541 van /home/usr1/sitename/public_html/sites/all/modules/file_entity/file_entity.file_api.inc).
The images are being displayed correctly, but they their alt tags are empty (
alt=""
), and like I said: admin/structure/file-types has nothing in it any longer. No tabs, no default types, whatsoever.Where could it have gone wrong?
Comment #192
HyperGlide CreditAttribution: HyperGlide commented@knalstaaf -- you applying this to the latest dev? existing site from your earlier posts? New site? Can you please give a bit more information.
Thank you
Comment #193
knalstaaf CreditAttribution: knalstaaf commentedRight, sorry. I updated the File Entity module of the same project mentionned in earlier posts, and applied patch 175 on it. I also updated the Media module with the latest dev and applied this patch. Applying the latter didn't go well, but I think the dev version of Media has that patch already applied to it.
Eventually I applied this patch to File Entity again to override the image styles (which is necessary for the website to show the pictures correctly).
I'm also getting this error in the Media "Select" window:
Warning: Invalid argument supplied for foreach() in views_handler_filter_file_type->get_value_options() (line 11 of /home/usr1/mysite/public_html/sites/all/modules/file_entity/views/views_handler_filter_file_type.inc).
Comment #194
knalstaaf CreditAttribution: knalstaaf commentedFixed!
To solve this, I had to go the the database. There I went to the table called "System". There I'd find the row "sites/all/modules/file_entity/file_entity.module". In that row there's a column called "schema_version". Putting the value to 7200 (instead of 7204) ànd running update.php again solved the issue. My former "file types" are back on the config page and so are the alt-values.
(Source: #1828872: Warning: Invalid argument supplied for foreach() in views_handler_filter_file_type->get_value_options() )
Comment #195
ParisLiakos CreditAttribution: ParisLiakos commentedAren't those reversed?
At least i only have token module, and it uses _ not -
Comment #196
HyperGlide CreditAttribution: HyperGlide commentedConfirm #195 token names are swapped with correct module.
Adjusted in the following patch.
Comment #197
Alex Andrascu CreditAttribution: Alex Andrascu commentedJust wondering...will this solution work for web / embeded too (e.g. youtube) ?
Comment #198
mgiffordCan we get this committed to the Repo? If not what additional changes are required?
Comment #199
ParisLiakos CreditAttribution: ParisLiakos commentedafter a few months and 200 comments here we go:
http://drupalcode.org/project/file_entity.git/commit/6ea2a18
Congrats!
Comment #200
Rob C CreditAttribution: Rob C commentedWoot! (200!)
Great work all!
Comment #201
Sk8erPeter CreditAttribution: Sk8erPeter commentedThank you very much for this feature!
Will there be any opportunities to use Token module to "autofill" the Alt and Title fields when the uploader user doesn't want to do this?
Just an example (I could imagine many other use cases): I would like to assign the
fid
to the Alt field automatically.Comment #202
supradhan CreditAttribution: supradhan commentedThanks a lot for this feature.
Can anyone work on it to make it work on any display format but not on just only 'Rendered File'. When I changed the display format to 'Shadowbox' or 'Image' there will no alt/title. If possible please let me know the hook to implement so that I can have title/alt in all display format.
Thank you.
Comment #203
BerdirPlease open a new issue for that. Looks like comment #185 has some code to get started. I think if that is made a bit more configurable (e.g. by using the same token-based configuration) and optional, then that would be a useful feature for this module. But again, in a new issue. 200 comments is enough.
Comment #204
lokz CreditAttribution: lokz commentedWill this be included in media galery beta9? when?
Comment #206
YesCT CreditAttribution: YesCT commentedAm i suposed to be able to do something with this like in #1885732-6: Documenting alt and title tags on image fields or was this just supporting getting the build in image field to work with alt and title, so I should not worry about configuring this?
Comment #207
HyperGlide CreditAttribution: HyperGlide commented@YesCT I would encourage you to open a new issue and if need be link to this as a reference. This was a long thread that I for one am happy to see 'fixed'.
Comment #208
Tritof CreditAttribution: Tritof commentedHi,
Great patch!
Is it possible to move to this version for a website built with the 7.X-1.2 version of media module with file entity bundled?
It seems to me that it is impossible.
Cheers
Comment #208.0
Tritof CreditAttribution: Tritof commentedChange reference to my name =)
Comment #209
justy CreditAttribution: justy commented196: 1553094-alt_and_title_support_for_images-196.patch queued for re-testing.
Comment #212
Devin Carlson CreditAttribution: Devin Carlson commentedComment #213
garvin CreditAttribution: garvin commentedI might be in the wrong place, can anyone tell me where to find a consensus on an Alt and Title patch for Media 7.x-1.3 ? Thank you!
Comment #214
jimkaprod CreditAttribution: jimkaprod commented196: 1553094-alt_and_title_support_for_images-196.patch queued for re-testing.
Comment #216
lawik CreditAttribution: lawik commentedAhahahahaha... This is hilarious.
I created this issue over 2 years ago. To try and help the Media and File stuff along a bit as part of a small hackathon. I'm still receiving notifications. Let's see if I can unregister from them now or if that feature is still missing.
Comment #217
gaele CreditAttribution: gaele commentedPlease open a new issue if you have any questions. (See #207)justy