Problem/Motivation
Alt and Title text that is entered is not carried over to the media [[...]] filter pattern and thus does not show up in the <img>
tag. This is a big problem for accessibility, and also for government sites, or other sites which require that.
Proposed resolution
The patch in #89 works and passes tests it applies to 7.x-2.x-dev.
It adds additional alt and title fields, with new help text, to the embedding form options section (where the "current format is" is set. Those fields get populated with the alt and title from the previous forms).
(The patch from #90 might apply to 7.x-1.2. It is similar to the patch from the duplicate issue #1343022-41: Adding alt and title attributes from fields to image markup which is reported to apply to 1.2.)
Remaining tasks
- (done/ongoing) update issue summary
- (done) steps to reproduce (see comment #95)
- (done) screenshots before (see screenshot 11. *no patch* in comment #95 and after (see screenshot 11. in #95, also see steps 13 and 15 in that comment)
- write tests (see comment #98 and http://drupal.org/node/1468170)
- (maybe) manual test for other kinds of media, like files or videos that these additions do not bother those types
- try patch from #90 and see if it applies to 7.x-1.2.
- decide if more manual testing is needed:
- manually test choosing the same image and re-using it in different content. See if it is possible to use different title/alts, while using the same image.
- manually test using an image field or a reference on the content
User interface changes
New help text and additional fields in a form, with values populated from other steps in the form.
API changes
No API changes.
Original report by @stella
I've read the documentation and looked at other similar reported issues, but can't seem to figure out how the hell to add alt or title attributes to an image uploaded via the WYSIWYG "media browser" button. Here's what I'm doing:
- Enabled media and media_internet modules
- Enabled the "media browser" button in my wysiwyg
- Enabled media input filter on the text format and ensured img tags weren't being stripped
- Created a new node of type "page".
- In the body field, clicked on the "media browser" button and uploaded an image from my local machine
- I then got the display size option but no place to add in alt or title text
- Saving the image then creates the media embed code but with the alt text attribute is empty
- Saving the node displays the image thumbnail, but again with no alt or title text.
I've seen suggestions elsewhere that I need to add an "alt" field to my "image" media field type, but having done that, nothing changed. I'm not embedding a previously uploaded image, nor do I have a media image field on any of my content types. I simply want to use the "media browser" to upload and embed images in textareas. However, the lack of alt and title attributes is preventing me from using this module at present.
Any suggestions?
Comment | File | Size | Author |
---|---|---|---|
#40 | 1307054-d7-2.patch | 2.1 KB | redndahead |
Comments
Comment #1
lecterro CreditAttribution: lecterro commentedComment #2
stella CreditAttribution: stella commentedI don't know why this was moved down to 6.x-1.x-dev, but I'm reporting this problem against 7.x-2.x-dev
Comment #3
amandine_m CreditAttribution: amandine_m commentedSuscribe
The problem is both in wysiwyg and when you use a file field as a media.
It's really a big trouble: the alt attribute is one of the most important thing in web accessibility.
I hope the dev gonna find a solution very fast, because I cannot use the module actually for this only reason...
Comment #4
Everett Zufelt CreditAttribution: Everett Zufelt commentedTweaking title.
IMO this functionality should exist by default, or there should be clear steps for enabling the functionality.
Comment #5
idflood CreditAttribution: idflood commentedFor the wysiwyg issue I may have a possible workaround. When the image is embeded in the wysiwyg right click on it and "Image properties" (or double click it). There you can define the alt, and in the advanced tab you can define the "Advisory Title".
But this doesn't solve the field issue and a better integrated UI would be great.
Comment #6
mgiffordBy default it sounds like the alt text should be added in the space where the user is prompted to enter the display size. I think it would be good to display the alt text by default and perhaps the title could be enabled through config for those who need it.
But yes, it really should be easier to do this by default.
Comment #7
idflood CreditAttribution: idflood commentedHere is a first patch that adds two textfields when inserting a media in wysiwyg. This is presented when selecting the format, and allows to set the alt and title. Obviously this needs some more work but I hope it's a step in the right direction.
Comment #8
jmcintyre CreditAttribution: jmcintyre commentedSubscribing, and... more detail on what sounds like a similar problem (and a contradiction to #5's recommendations above):
If I double-click an uploaded image, the "Alternative text" field in the Image Properties contains the same number that's used in the inserted markup (shown as "XX" here):
[[wysiwyg_imageupload:XX;]]
If I edit the ALT text field and replace the XX with anything else, the image is no longer displayed once the node is saved.
Comment #9
Lukas von BlarerI think this is a crucial issue. For what reason are these fields not created by installing the Media module?
The patch is not working for me at all. No fields do appear on my file field with the Media browser widget.
I am also missing some place where I could easily administrate Files. The only thing we are able to do currently is deleting files.
Shouldn't we change the category and priority of this issue?
Comment #10
idflood CreditAttribution: idflood commentedIf you test patch in #7 be sure to clear the drupal and browser caches. There is a modification on a javascript loaded from the iframe and this require a browser clear cache.
Comment #11
kirilius CreditAttribution: kirilius commentedIs there any chance this gets rolled into a release?
Comment #12
Lukas von BlarerSorry, my fault. I thought it was for both WYSIWYG and the Media field. Would it be difficult to implement this for the Media browser widget as well?
Comment #13
BarisW CreditAttribution: BarisW commentedBumping the priority. This bug rules out using Media on all governmental websites as alt tags are mandatory.
Comment #14
BarisW CreditAttribution: BarisW commentedSee this issue for a possible solution: #1332500: Alt text doesn't work from wysiwyg
Comment #15
kirilius CreditAttribution: kirilius commentedThe issue linked in #14 has a link back to this current issue ;-) I guess more and more duplicates of the same issues will pop up soon as title/description and alt attributes from the original ImageField are very widely used. Media should support them as well.
Comment #16
mgiffordPatch in #7 applies nicely to HEAD but I haven't tested out the module yet to see how well this applies.
Agreed that this is a critical issue. Alt tags are low hanging fruit for accessibility of any site, but also important for SEO.
Comment #17
Daniel KorteI've updated the patch in #7 to not include the title attribute in the JS function forceAttributesIntoClass. Otherwise every word except the first word in the title attribute is added to the class attribute.
Comment #18
jonathanmd CreditAttribution: jonathanmd commentedHere's how I solved the problem for the media widget (doesn't solve the WYSIWYG issue.)
http://drupal.org/node/1343022
Comment #19
mgiffordThere's also http://drupal.org/project/eim
But ultimately this would just be part of the Media module.
Comment #20
dinakaran.ilango CreditAttribution: dinakaran.ilango commentedhello guys
i am using media module to insert images in wysiwyg editor my input format is filtered html..when i insert images using add media in wysiwyg it does.but when i view the page instead of images i am seeing something like
[[{"type":"media","view_mode":"media_original","fid":"27","attributes":{"alt":"","class":"media-image","typeof":"foaf:Image"}}]]
i dont get it please help me
Comment #21
Daniel KorteDo you have "Converts Media tags to Markup" checked for your Text Format at /admin/config/content/formats/?
Comment #22
idflood CreditAttribution: idflood commentedI've tested the patch in #17 and it works really well. This fix the issue of broken title I had with the patch in #7.
Comment #23
dinakaran.ilango CreditAttribution: dinakaran.ilango commentedthanks for your reply that solved my problem
Comment #24
dqd@ #18 jonathanmd: Your solution is a great idea but doesn't show any markup. Did you test it under different scenarios already?
Comment #25
jonathanmd CreditAttribution: jonathanmd commented@Digidog
What type of markup are you looking for, HTML of the image? There's essentially no change to the image tag other than adding an alt tag or title tag.
All other HTML markup is the same.
Comment #26
dqd@ jonathanmd : uhm , yeah :) Look, Which part of markup you are hoping to change with your solution isn't really heard to get. :) When I talk about markup in this context, of course I only talk about the awaited alt="" part of the markup and nothing else. I thought this would be clear? Don't get me wrong, but I am not looking for a solution for myself. I just follow the discussion to find the point to start from patching against media or any other module and I just wanted to inform you that it doesn't work over here to help you with report.
But as I already sad before, I don't think, that this, nor the conceptual decision, which it is based on, is the right way for any case. A more flexible point to attach alt="" and title="" like attributes to entities should be preferred. I am not bothering your work-around. I talk about the point it attaches to. In the moment I think about a concept near to Dave Reids field injector to turn any text field to an attribute field which can be injected in any case like -img- or -a- or similar. As a maintainer of link module I would love to see a solution which is DRY and can help more than one situation.
Maybe there should be two points it can be made from @ UI, where the second point, when the image gets inserted into a node will override the first point, where the attributes are set directly to the file entity before... Just thinking ... ... Just thinking ...
Comment #27
mediameriquat CreditAttribution: mediameriquat commentedPatch #17 works fine in wysiwyg.
A similar patch would be useful when images are published as fields. Any help with that would be greatly appreciated.
In the meantime I'll use the classical image widget, populate the alt+title fields, and switch back to the media browser later (changing the widget doesn't destroy the data).
I wouldn't recommend the solution in #18, because the alt and title fields may vary according to the node type or node language. Sticking to what's already implemented in core is still the safest option, as I have 100s of images to publish.
Comment #28
shaisamuel CreditAttribution: shaisamuel commentedCan we get these patches and #18 to be added to the media project? Or they need to stay a private solutions which need to be applied on every new release?
Comment #29
Dave ReidComment #30
shaisamuel CreditAttribution: shaisamuel commentedThank you Dave Reid, but my question is about Alternative Text with the regular widget and not the WYSIWYG.
Comment #31
dqdshaisamuel:
... I think the issue lies deeper than @ media, and Dave tries to get down to the root of the trouble.
Look here #1291262: Add 'alt' and 'title' tokenized text options for image formatters, and a 'title' option for the generic file formatter
I think that's why he moves this here to a rather soluble issue task ... but not sure yet ...
Comment #32
maclore CreditAttribution: maclore commentedI'm new to drupal and wondering how I'll use this patch. Can you be of help please. Thanks.
Comment #33
tcbun CreditAttribution: tcbun commentedI agree this functionality should be in the Media module itself, but, until it is I use EIM (http://drupal.org/project/eim) which was mentioned in comment #19.
Comment #34
jwilson3It would be great if the field names and descriptions matched the proposal here: http://drupal.org/node/1343022
Comment #35
Reg CreditAttribution: Reg commentedI had an issue with views where a content type of image being used in a view wouldn't validate as proper HTML.
Looking into why it came down to using the media widget instead of the image widget where the alt tag was never set. I'm assuming that by skipping it that the field in the database got left as NULL and therefore no alt tag was pulled through the view.
The bottom line is that by removing this functionality it caused problems elsewhere in the system so the ability to set the alt and title tag really needs to be brought back into this module as a standard feature.
Comment #36
mfer CreditAttribution: mfer commentedAttached is an update that adds the descriptions form #34. There is also an if statement added so we should only show this form if the file is an image. Videos, for example, don't have alt text. The #default_value of '' was removed because code sets that as the default if not supplied.
Can ya'll review?
Comment #37
redndahead CreditAttribution: redndahead commentedThis may be being handled at #1553094: Alt and Title support for Images or we may need an integration between the two. Either way I think the other issue probably should be committed first.
Comment #38
redndahead CreditAttribution: redndahead commentedI just posted a comment on #1553094: Alt and Title support for Images that tried to lay out a plan for integrating alt and title tags. It would be great if people could comment on the validity of that approach.
Comment #39
mfer CreditAttribution: mfer commented@redndahead I agree with your flow. Seems sane.
Comment #40
redndahead CreditAttribution: redndahead commentedHere is an updated patch that adds support for fields already created in file_entity module.
See the patch located here: http://drupal.org/node/1553094#comment-6082736
Comment #41
redndahead CreditAttribution: redndahead commentedI created a video, no sound, of what the workflow looks like with these patches.
http://youtu.be/4kVcyY2soDo
Comment #42
yatil CreditAttribution: yatil commentedComment #43
omar CreditAttribution: omar commentedI installed and tested the patch in #40. The extra step for adding the alt/title texts is indeed added to the process of adding an image into the node body using the Media Browser button. The test submission was successful and the image contained the alt/title texts in source.
However, this issue also affects images added as file field entries via the "Media file selector". So, as far as I can tell, this is only a partial fix of the issue... unless we break in into two parts, one "RTBC" and the other "needs work".
Omar
Comment #44
mgiffordThe patch no longer applies to the master branch.
$ git apply 1307054-d7-2.patch
error: includes/media.filter.inc: No such file or directory
error: js/wysiwyg-media.js: No such file or directory
Comment #45
mrfelton CreditAttribution: mrfelton commentedThe patch seems to apply fine to the latest dev code, however #43 is right - this solves the problem when using the WYSIWYG browser, but does nothing for meda uploaded to a file field.
Comment #46
Pomliane CreditAttribution: Pomliane commented@omar and @mrfelton: see #1553094: Alt and Title support for Images for the field part of this issue.
@redndahead: not sure if it comes from this patch or #1553094: Alt and Title support for Images but the WYSIWYG integration seems only partly solved by #40.
In the workflow of http://youtu.be/4kVcyY2soDo alt/title fields are filled just after upload (~@ second 30) and everything works as expected.
Editing alt/title fields after that, has no effects: neither alt/title show the edited text in the current content—initial entries are displayed, better than nothing :)—nor the file itself is modified.
Editing alt/title for the current content through the WYSIWYG editor itself (WYSIWYG + CKEditor) has no effect either.
Comment #47
mgiffordWhat needs to happen to bring this into the next release of this project? It's been inactive since July.
Comment #48
ParisLiakos CreditAttribution: ParisLiakos commented@mgifford, first step would be to reroll #46 against latest dev
Comment #49
mgiffordWell, there's 2 branches, right. If someone does it might as well be against the git repo, right? --branch 7.x-2.x I expect.
Comment #50
opdaviesSubscribing
Comment #51
pyxio CreditAttribution: pyxio commentedYes, this is very unfortunate :o( I was very surprised to not have any ALT or TITLE field in the media selector widget. Otherwise, great work on this module.
Cheers
Kevin
Comment #52
pyxio CreditAttribution: pyxio commentedI just watched this video. Seems like a temporary solution. My question is how do you get back to the ALT and TITLE fields after saving your data initially? For example, is it easy to go back and change the data?
Cheers
Kevin
Comment #53
supradhan CreditAttribution: supradhan commentedIs there any Patch or Fix for this issue. I am using media 2.0 and it is unfortunate to see media module doesn't have alt and title support. Also it would be a good feature to give user to edit the file name.
Comment #54
HyperGlide CreditAttribution: HyperGlide commented@CIsSharp all discussion to my knowledge relies on the 2.x branch -- please look here at a patch #1553094: Alt and Title support for Images
Comment #55
supradhan CreditAttribution: supradhan commented#54 @HyperGlide: Sorry about that. I will change it to 2.x. I tried with latest version of Media (7.x-2.0-unstable6+39-dev) and file entity (7.x-2.0-unstable6+13-dev) without success. It allows to add alt tag and to change the file name but it is not working.
Comment #56
Homotechsual CreditAttribution: Homotechsual commentedIs this in the latest dev release?
If not what needs to be done - I'm prepared to help, such a basic feature missing from "THE" drupal media module is surprising.
Without this addition it's impossible to get media module to output valid code as the "alt" attribute is a required attr for html5 validation at the very least.
Let's get this in 7.x-2.x DEV and out there ready for the next beta/unstable.
Comment #57
HyperGlide CreditAttribution: HyperGlide commented@MJCO thanks for the offer of help.
I believe first we need to complete #1553094: Alt and Title support for Images then re roll and retest this patch.
Please read over that long issue queue and if there are any questions please post em.
Thank you!
hg
Comment #58
dshields CreditAttribution: dshields commented#1553094: Alt and Title support for Images is RTBC for file_entity.
Looks like its time to push forward with this. Any assistance would be appreciated as Alt and Title really are critical attributes of an img tag for many users.
Comment #59
mgiffordRemember - perfect is the enemy of good. Right now we've got a bad solution for everyone wanting to make their sites accessible. Lots of folks have moved to Drupal 7 to make their sites more accessible and alt tags is totally the low hanging fruit of accessibility.
Let's get something good into a release and if we need to perfect it we can do so in a following thread. This thread is over a year old now.
Comment #60
sylus CreditAttribution: sylus commentedI agree with the above. Is there a list of the current problems not being addressed from the earlier patch other then porting it to the latest dev?
Comment #61
mrfelton CreditAttribution: mrfelton commentedI tried porting the patch, but the javascript has changed so much since the patch in this issue was written that its now unrecognisable, and hard to figure out from the context of that patch where the changes to the javascript now need to be made.
Comment #62
mgifford#40: 1307054-d7-2.patch queued for re-testing.
Comment #64
dshields CreditAttribution: dshields commentedAny development on this issue yet?
Comment #65
jpstrikesback CreditAttribution: jpstrikesback commentedSee: #1451316: Clean up wysiwyg-media.js
From a quick read I'm not sure we need to hammer attributes into wysiwyg-media.js anymore.
Comment #66
mgiffordJust bringing forward the patch in #40 against 7.x-1.x-dev knowing that I haven't changed the version number and it's going to bust.
Comment #67
mgifford@jpstrikesback not sure how we'd hammer them in at this point. This patch doesn't include any changes to wysiwyg-media.js and is built against the 2.0 git repo.
Comment #68
jpstrikesback CreditAttribution: jpstrikesback commentedTested and unfortunately changes to existing/new alt/title text does not get picked up...Tho it is nice to see alt & title working with any file you upload, but they cannot currently be changed via the media browser form in wysiwyg
Comment #69
jpstrikesback CreditAttribution: jpstrikesback commentedOK, found the spot in wysiwyg-media.js that needed attributes added back in. Please test the patch attached, there is work to be done in the media format form because it never pulls any settings out of the tag when editing an already inserted item, that can probably go into a new/existing issue. This get's us the ability to set alt/title per image & override what is set in a field alt/title :)
To see the limitations, add or modify an alt/title and then try to edit it via the media popup again. You'll notice the text in the token does not make it back to the format form.
Comment #70
jpstrikesback CreditAttribution: jpstrikesback commentedThat one in #69 wont apply/pass, my git settings are funny and I have to manually edit the file paths...this one should apply:
Comment #71
jpstrikesback CreditAttribution: jpstrikesback commentedComment #72
jpstrikesback CreditAttribution: jpstrikesback commentedtrying to reset the testbot
Comment #73
jpstrikesback CreditAttribution: jpstrikesback commentedSomething is up with the testbot...
Comment #74
ParisLiakos CreditAttribution: ParisLiakos commentedgot a pretty big queue to handle
http://qa.drupal.org/pifr/queued
Comment #75
sylus CreditAttribution: sylus commentedA comma is missing after view_mode: formatted_media.type
This will cause the wysiwyg js to fail.
Thanks for pushing this issue forward!
Comment #76
jpstrikesback CreditAttribution: jpstrikesback commented@sylus, it actually doesn't and is important as this bit is in a JavaScript file, where a trailing comma can mess with IE (Internet explorer)Edit: your right!!! nice catch!!
Comment #77
sylus CreditAttribution: sylus commentedHmm are you sure your patch has:
I think it should be
Once I add the comma I no longer get a JS error and TinyMCE loads
Comment #78
jpstrikesback CreditAttribution: jpstrikesback commentedof course, I misread your comment sorry, I thought you were saying .options needed a trailing comma! :)
Comment #79
jpstrikesback CreditAttribution: jpstrikesback commentedThere we are...one too many command Z's to remove console.log(blah) got me in that spot :) Thanks @sylus!
Comment #80
jpstrikesback CreditAttribution: jpstrikesback commentedaaaaaand one more time with a folder structure that will pass...argh
Comment #81
sylus CreditAttribution: sylus commentedThanks!
With this patch I think I finally have alt working with media.
So with patches:
1) #79 from this thread
2) #1553094: Alt and Title support for Images or latest release from File_Entity
and in hook_wysiwyg_editor_settings_alter for tinymce
I now finally have working alt and title tags! :)
Comment #82
jpstrikesback CreditAttribution: jpstrikesback commentedExcellent!! Let's get a few more testers on it and RTBC this baby!
Comment #83
jpstrikesback CreditAttribution: jpstrikesback commented#80 passed, hopefully the testbot will catch up and send results
Comment #84
jpstrikesback CreditAttribution: jpstrikesback commentedSomewhere the values are getting stripped from the alt & title tags in the final render on a published node, they make it thru when the alt/title is in the alt/title field, but not when it's only in the media tag, they still exist but are not rendered, so somewhere there is some validation going on I guess
Comment #85
wrd CreditAttribution: wrd commentedI'm getting this as well. The patch in #80 works so far as letting me enter Alt and Title text, and they appear in the attributes in the media tag:
[[{"fid":"620","view_mode":"media_original","type":"media","attributes":{"height":220,"width":220,"alt":"Alt Test","title":"Title Test","class":"media-element file-media-original"}}]]
...but the rendered image tag is missing those attributes:
<img height="220" width="220" alt="" title="" class="media-element file-media-original" typeof="foaf:Image" src="/sites/default/files/u2/attraction_test.jpg">
One thing I didn't do was add the attributes to hook_wysiwyg_editor_settings_alter() -- I'm using CKEditor rather than tinymce, so I wasn't at all sure if I needed to do something similar. Anyone been able to get this to produce alt and title attributes on output?
Comment #86
jpstrikesback CreditAttribution: jpstrikesback commentedI think the issue is in media.filter.inc media_token_to_markup() when it calls media_get_file_without_label() to prepare the item for render, it doesn't have a way to send in the attributes cause it renders it from the file entity properties...tho I haven't looked at this in almost a week, someone please correct me if I'm wrong
Comment #87
ParisLiakos CreditAttribution: ParisLiakos commentedcant you override the file object values before passing it to media_get_file_without_label ?
Comment #88
wrd CreditAttribution: wrd commentedI'm not certain why this works, and by no means am I certain this is the right thing to do it, and I have no idea how to offer it as a patch, but this seems to get my alt and title tags filled in on rendered nodes. Starting at line 179 of media.filter.inc:
Comment #89
jpstrikesback CreditAttribution: jpstrikesback commentedThanks @rootatwc & @wrd, that's exactly what was needed. I used what had already been cleared for landing in
$settings
and placed in the$file
object. I also did an !empty() instead of isset() so as not to override a fields default alt/title with an empty alt/title in the edited textarea field.Comment #90
avenhe CreditAttribution: avenhe commentedHere is my patch
Comment #92
jpstrikesback CreditAttribution: jpstrikesback commentedHey @avenhe, can you add a description of your changes (why media.fields.inc, etc) and check your patch applies against current dev with "git apply"?
Cheers,
JP
Comment #93
YesCT CreditAttribution: YesCT commentedneeds re-roll and interdiff for #90 patch.
also needs issue summary updating, and steps to reproduce
Comment #94
jpstrikesback CreditAttribution: jpstrikesback commented#89: 1307054-d7-2-alt-text-89.patch queued for re-testing.
Comment #95
YesCT CreditAttribution: YesCT commentedwith the patch from #89:
0. get drupal and modules
get d7, (I did git clone)
cd drupal/sites/all/modules
get these modules (I did git clones, but could have downloaded tars, or done drush dl)
git clone --recursive --branch 7.x-2.x http://git.drupal.org/project/media.git
git clone --recursive --branch 7.x-2.x http://git.drupal.org/project/file_entity.git
git clone --recursive --branch 7.x-3.x http://git.drupal.org/project/views.git
git clone --recursive --branch 7.x-2.x http://git.drupal.org/project/wysiwyg.git
git clone --recursive --branch 7.x-1.x http://git.drupal.org/project/ctools.git
cd media
curl -O http://drupal.org/files/1307054-d7-2-alt-text-89.patch
git checkout -b media-89
git apply --index 1307054-d7-2-alt-text-89.patch
git commit -m "89"
1. enable modules: wysiwyg and media
(dependencies also cause enabling: File entity, Chaos tools, Views)
2. configure wysiwyg admin/config/content/wysiwyg
download, unzip wysiwyg (I did tinymce and ckeditor, an older version) and put in correct spot in libraries (make libraries directory)
3. pick ckeditor for full html (could have done filtered html, but would then have to add
<img>
[edit: add code to see the img tag] to the allowed tags)4. edit (in that row) and check media browser
get error have to enable media filter "Convert Media tags to markup". do that.
check the media browser button again. saves fine this time.
5. create a page
6. upload an image
7. next
8. form lets enter alt and title
9. save
10. save again
10 *no patch*. save again
11. pick format (if picking default, had to have set the default image file to image at admin/structure/file-types/manage/image/file-display, I'll just pick preview)
11 *no patch*. pick format (preview)
12. see image in the wysiwyg text area preview
13. can toggle the disable rich text to see the html/filter
<p>[[{"fid":"2","view_mode":"teaser","type":"media","attributes":{"height":137,"width":220,"alt":"test jing capture of wysiwyg settings","title":"Test Capture of WYSIWYG word","class":"media-element file-teaser"}}]]</p>
13 *no patch* toggle
shows something like:
<p>[[{"fid":"3","view_mode":"teaser","type":"media","attributes":{"height":137,"width":220,"class":"media-element file-teaser"}}]]</p>
14. save
15. image shows fine. tool tip works. can inspect the html to see
<img alt="test jing capture of wysiwyg settings" title="Test Capture of WYSIWYG word" class="media-element file-teaser" typeof="foaf:Image" src="http://localhost:8888/d7-git/sites/default/files/styles/medium/public/test_0.png" height="137" width="220">
15 *no patch*. image shows fine. no tool tip. inspect and see no alt or title tag
<img class="media-element file-teaser" typeof="foaf:Image" src="http://localhost:8888/d7-git/sites/default/files/styles/medium/public/test_1.png" alt="" title="" height="137" width="220">
Comment #96
YesCT CreditAttribution: YesCT commentedI looked into applying the patch in 90 and I'm not sure what happened to corrupt it.
This is the changes from it (I think).
I did an interdiff, but it probably does not make much sense... it's here anyway.
I did not test 90, or do a code review.
@avenhe Let us know your thoughts on why your changes from 90 might be a better approach...
Comment #97
YesCT CreditAttribution: YesCT commentedlooking at the redid-90.patch from #96
why was this alt section moved to media.fields.inc from the media.filter.inc file? Does it better get a default value? What is the concern the Notice is addressing?
why was this section moved to media.fields.inc from the media.filter.inc file? Does it better get a default value?
does this code block accomplish the @todo of making it work for file and image fields?
This function needs a doc block.
Comment #98
YesCT CreditAttribution: YesCT commentedI tried the redo to 90 patch from #96
It acts like 7.x-2.x without the patch, like screenshot 11 no patch in #95 and the title and alt is not in the media [[..]] filter string.
Based on that, I recommend the patch from #89 and I'll mark it rtbc for 89.
Does the maintainer require tests? If so, mark this back to needs work.
It would be really nice if a test were added so that we could show it fails to add the alt and/title without the patch.
http://drupal.org/node/1468170 [edit: forgot the actual link. adding it.]
This doc page is for adding tests to core, but would be helpful for how to add tests for this, and also it shows how to make a tests only patch and a complete patch that will show pink and then green. This exposes the bug and proves the fix works.
Comment #99
YesCT CreditAttribution: YesCT commentedUpdating the issue summary.
Comment #99.0
YesCT CreditAttribution: YesCT commentedupdated issue summary and used issue summary template
Comment #100
ParisLiakos CreditAttribution: ParisLiakos commentedawesome job, thanks a lot YesCT..although i dont require tests, it would be good to start adding some now that testbot is back to life for media..
If someone cares to add some before i commit this, i will definitely not say no
Comment #101
jpstrikesback CreditAttribution: jpstrikesback commentedYou rock YesCT!
Comment #102
jwilson3@YesCT: Job well done. Thank you, Thank you, Thank you! :)
Comment #103
YesCT CreditAttribution: YesCT commentedThere is some more work here to do I think.
A kind person alerted me that this very same approach was taken out, on purpose. So we need to look into that more to see why and what the alternative suggestion is.
Also, we should look at the 1.x branch and see if we can port this (or the inferred better way) back to that.
:)
We'll get there eventually. :)
Leaving it rtbc until we have specific feedback that says this approach is a won't fix (hopefully linking to the issue or commit that took it out) and/or a needs work with the approach that should be used instead.
Comment #104
kirilius CreditAttribution: kirilius commentedIf this patch works, can it be added to a release any time soon?
Comment #105
jpstrikesback CreditAttribution: jpstrikesback commentedYesCT can you elaborate? I'm in favor of this going in now for a major accessibility win and then being improved if a generalized 'extra rendered property' feature request comes forward afterwards...
Comment #106
rcross CreditAttribution: rcross commenteddo any of the patches above work with the 1.x branch? is there a work-around for 1.x?
Comment #107
dwatts3624 CreditAttribution: dwatts3624 commentedPatch #89 works great! I like how this associates the data with the file entity so it can be reused. Well done!
Comment #108
YesCT CreditAttribution: YesCT commentedSee related discussion: #1343022-54: Adding alt and title attributes from fields to image markup
Comment #109
joelcollinsdc CreditAttribution: joelcollinsdc commentedi concur with comment 55 in that thread. the solution here is actually very elegant, the alt/title are indeed stored along with the media, but they are merely suggestions during placement of the image that can be overridden on display.
Comment #110
YesCT CreditAttribution: YesCT commentedI'm still sorting out all the related issues and approaches.
The patch from #90 (and thus #96 which is discussed in #97) is mostly the same as the patch in the duplicate issue #1343022-41: Adding alt and title attributes from fields to image markup which applies to 7.x-1.2.
In #1343022-54: Adding alt and title attributes from fields to image markup, Frank points out that the media module is removing the image field alt/title fields.
// On image fields using the media widget we remove the alt/title fields
http://drupalcode.org/project/media.git/blob/93b4d7134b6da41751ef330a391...
I'm guessing that this is because using the media widget and image fields, images can be reused, and alt and title should be able to be different each time it's used to display an image. <- I dont think that is quite accurate, so please correct me.
Still more thinking to do on this.
Comment #111
jpstrikesback CreditAttribution: jpstrikesback commentedThanks YesCT,
This is the case with the patch in #89 for WYSIWYG usage, i.e. it allows one to add a new alt/title to each instance of a image in wysiwyg. You are correct that this is not the case for usage in a field as then it uses the field formatters and storage IIUC.
Regarding the 7.x-1.x branch I believe we can focus on adding new features to 2.x and backporting later.
Cheers,
JP
Comment #111.0
jpstrikesback CreditAttribution: jpstrikesback commentedadd a possible remaining task of testing with other kinds of files.
Comment #111.1
YesCT CreditAttribution: YesCT commentedupdated with info about a workaround for 1.2 and drafts of manual test that might be needed.
Comment #112
YesCT CreditAttribution: YesCT commentedI updated the issue summary. And I guess we are (I am) kind of done here until we get someone to commit this or mark it needs work.
To clarify, the remaining tasks in the issue summary are "extra" and might help but I do not think they are urgent or required.
Comment #113
jpstrikesback CreditAttribution: jpstrikesback commentedAgree :) and thanks for all your work here YesCT!!
Comment #114
DamienMcKennaMight it not be better to allow the file's alt & title text be assigned but allow them to be overridden per usage? This would cover for all three possible scenarios - one set of alt/title tags per image, "default" alt/title tags per image that may be overridden per use, no "default" tags per image forcing them to be set per use.
Comment #115
jpstrikesback CreditAttribution: jpstrikesback commented@DamienMckenna,
#89 allows any previously stored alt/title (i.e. in field storage, Tho that is not per instance is it, that doesn't seem to make sense??) to come thru as the default, but allows one to also override those defaults on an individual instance basis when used in a textarea via WYSIWYG media integration.
Comment #116
ParisLiakos CreditAttribution: ParisLiakos commentedremoved t()
and commited
Comment #117
DamienMcKennaAwesome work everyone!
Comment #118
jpstrikesback CreditAttribution: jpstrikesback commentedNice! Thanks all!!
Comment #119
kirilius CreditAttribution: kirilius commentedThanks, I needed this a lot! A couple of questions:
1) Is this going to be rolled into e a release soon?
2) I have already a few hundred image files uploaded to my site. I also have them populated into some Image Fields which today don't have alt and title attributes. After I update Media with this fix will my existing images get alt and title automatically? If not, what do I need to do in order to synchronize my files' and my images' alt and title attributes?
Comment #120
HyperGlide CreditAttribution: HyperGlide commented++ to all!
Comment #121
fcarneiro CreditAttribution: fcarneiro commentedI would also like to know the answer to #119-2
I know it will not be automagically ;)
Comment #122
Frank Ralf CreditAttribution: Frank Ralf commentedSorry for re-opening this issue but I upgraded to the current dev version of Media 2 (7.x-2.x-dev 2013-Jan-06) and notices some strange behavior.
I wonder whether this is the intended behavior.
Comment #123
arthurf CreditAttribution: arthurf commented@Frank Ralf - this is specific for files fields- not for image fields. The screen shot that you show above is from an image field so you won't see the functionality described in this thread.
Comment #124
ParisLiakos CreditAttribution: ParisLiakos commentedActually to be more correct.that functionality is for file/image fields that use the media widget.
Those settings come from core image module, but right now media widget does not use them.
Instead, fields attached to image file type provided by file_entity module are used.
If you dont have those fields attached to your image file type, plz make sure you have latest file_entity version and run update.php
Comment #125
YesCT CreditAttribution: YesCT commentedI did a quick test.
Yep. definitely via the media browser in wysiwyg is different than an image in an image field on the content type.
I'm going to make sure I have everything updated and do a few more tests.
Comment #126
YesCT CreditAttribution: YesCT commented@rootatwc
what do you mean "those fields"? the ones for the media browser wysiwyg? (or the ones for the core image field?)
Comment #127
YesCT CreditAttribution: YesCT commentedopened related #1885732: Documenting alt and title tags on image fields (so as to not clog up this wysiwyg issue with general investigation on my part.)
@Frank Ralf is what you saw in #122 what I saw in comment 3 there? #1885732-3: Documenting alt and title tags on image fields
Comment #128
ParisLiakos CreditAttribution: ParisLiakos commentedThose fields = Alt and title field.
See under Structure -> File types -> Image -> Manage fields
Comment #129
Frank Ralf CreditAttribution: Frank Ralf commentedThanks for testing and clarification!
Will follow the issue at #1885732: Documenting alt and title tags on image fields.
Comment #130
Frank Ralf CreditAttribution: Frank Ralf commented@rootatwc (#124)
Thanks for this information, that was the missing piece of the puzzle.
Is there an upgrade path to convert core Image fields with their alt and title tags to Media 2's file entities?
Comment #131
ParisLiakos CreditAttribution: ParisLiakos commentednot, that i am aware of, but wouldnt be that hard to create one
Comment #132
Frank Ralf CreditAttribution: Frank Ralf commentedThanks for the quick reply!
Comment #134
kirilius CreditAttribution: kirilius commentedRe-opening the issue because of the lack of upgrade path... ot whould I open a new one for that?
Comment #135
ParisLiakos CreditAttribution: ParisLiakos commentedI would rather call it migrate path.Please open a new issue..
Comment #136
samwillc CreditAttribution: samwillc commentedI'm using the latest media 2x and file_entity and I can't see any alt and title fields anywhere.
I have a 'File' field, using media file selector.
At /file-types/manage/image/fields
I have 'File name' and 'File' fields only
At /node/8/edit
I have Select Media|Edit media|Remove media
If I select 'Edit media', a little popup with 'Name' and 'Replace file'.
At /file-types/manage/image/file-display
I have 'Image style' (dropdown), 'Alt attribute' and 'Title attribute' fields
So where are these fields actually displayed so I can fill them in when I upload an image? I can't seem to find them.
Comment #137
Frank Ralf CreditAttribution: Frank Ralf commentedI also missed those fields at first. They will show in one of the subsequent steps when choosing a file with Media's "Media file selector" widget.
hth
Frank
Comment #138
samwillc CreditAttribution: samwillc commented@Frank Ralf, yes they do now?! Maybe I didn't have the latest version after all. Thanks.
Comment #139
MatsSvensson CreditAttribution: MatsSvensson commented"closed fixed"?
So is this fixed or not?
If it is, where are alt & title in the actual UI?
Can i JUST use it, or do i have to follow some "one meelion simple little steps"-description?
what is the official information about this?
Comment #140
arthurf CreditAttribution: arthurf commentedHi MatsSvensson-
Sounds like you're frustrated. Unfortunately no one here is paid to maintain this module so we all try to pitch in where we can. A number of people have worked on a fix for this and as it currently stands this functionality works correctly in the 2.x branch. I can't speak for the 1.x branch as I'm not familiar with it.
There is documentation that is referenced from the project page. If you don't find it adequate please file an issue with your suggestions or feel free to add to it.
Comment #141
MatsSvensson CreditAttribution: MatsSvensson commentedBut can i use 2.0?
It says "unstable".
Its pretty confusing to have to choose between a "recommended" release that obviously does not work, and a "unstable" release that everyone says works.
Comment #142
jpstrikesback CreditAttribution: jpstrikesback commentedHi MatsSvensson,
Granted that would be confusing but it is also good to remember that unstable does not equal broken or non functioning, it means it is not stable, APIs, functionality, UX, etc are subject to change without notice (tho in practice you will find plenty of notice in the issue queues).
As to whether you can use it or not, that is kinda dependent on your plan for ongoing maintenance of whatever site you install this in. I am, but that means I have to keep track of changes and re apply patches, watch for functionality removed, whatnot, while I wait for a stable version.
Best,
JP
Comment #143
samwillc CreditAttribution: samwillc commentedHi MatsSvensson,
I am using:
File entity: 7.x-2.0-unstable7
Media: 7.x-2.0-unstable7
For your benefit, I knocked up a very quick image with a few screenshots of the stages involved for uploading an image. This is what happens on my new local site for a simple product gallery.
http://dl.dropbox.com/u/63070476/media2-demo.png
1) Click 'Select media'
2) Browse to file
3) Click 'Submit'
4) Gives you a preview of the image and the elusive 'Alt' and 'Title' fields.
5) Save
6) Not pictured, but after the image is uploaded, when you click 'Edit', you get another popup where you can edit the 'Alt' and 'Title'.
Give it a shot with these versions, see how you get on.
I hope this illustrates things a bit more clearly.
Comment #144
MatsSvensson CreditAttribution: MatsSvensson commentedGreat info!
Thanks for the help.
Ill give it a try when i get some time for more testing.
Comment #145
moonray CreditAttribution: moonray commentedFollowing the steps in #143, I never get 4)
After Submit in 3) it just closes the media popup without ever showing the Alt and Title fields.
This is new since updating media module from 7.x-2.0-unstable7+2-dev to 7.x-2.0-unstable7+38-dev. Before that I had the alt & title fields.
EDIT: Looks like I needed to update file_entity module to latest dev as well. Now they are showing up again.
Comment #145.0
moonray CreditAttribution: moonray commentedUpdated issue summary fixed [# problem
Comment #146
snaushads CreditAttribution: snaushads commentedCan we use the patch on a Stable version of Media 1.3. How do we apply the patch ? I am new to the concept of Patching drupal modules, I would prefer manual patching.
Id appreciate any guidance. Thanks.
Comment #147
stinky CreditAttribution: stinky commentedI just updated to Media 7.x-2.0-alpha2 and I have the current version of File_entity 7.x-2.0-alpha2.
I'm still missing my Alt/Title field data. Any help on getting this data back is appreciated. I'm working on a big site and we need the Alt/Title field tags!
Thanks,
Stinky
Comment #148
kaidjohnson CreditAttribution: kaidjohnson commentedAnyone still experiencing this issue should join the testing/discussion at https://drupal.org/node/2067063.
Comment #148.0
kaidjohnson CreditAttribution: kaidjohnson commentedsmall typo fixed
Comment #149
juampynr CreditAttribution: juampynr commentedI know this issue was closed but since #2142571: Spin-off WYSIWYG support in dedicated module / project got rid of all of this code I have nowhere else to reference it.
The commit added at #116 uses 'safe_value' instead of the raw one. This encodes symbols like double quotes and shows them in the form field. See the following attachment:
This makes that the json representation of the image saved in the database is invalid and therefore the image is not rendered.
Just for reference for people who still have this version of the media module, here is a patch to address this issue.
Comment #150
drupov CreditAttribution: drupov commentedI am using media 7.x-2.0-alpha3+46-dev and cannot apply the patch from #89. I get this.
In my dev version (7.x-2.0-alpha3+46-dev) I don't have the file includes/media.filter.inc
This file exists in media 7.x-2.0-alpha3, but the patch from #89 fails there also:
What is the situation with this patch for 7.x-2.x?
Thanks!
Comment #151
HyperGlide CreditAttribution: HyperGlide commented@drupov this issue is closed and the patch was committed please look at
https://drupal.org/comment/6877824#comment-6877824
If have other questions suggest you open a new issue.
Comment #154
joseph.olstadpatch from comment #40 is still good, still applies
review this and get it in asap.
#1307054-40: Accessibility - Media browser image alt and title fields
Comment #155
joseph.olstadComment #156
ndf CreditAttribution: ndf at Dx Experts commentedThe functionality is committed and available in the stable 7.x-2.x branch. (today 7.x-2.13)
See comment #116.
Behavior:
1. You upload an image with the media browser.
2. You can then set optional title and alt tags.
3. When the image is rendered, the alt- and title-tags are rendered.
#155, #122
Can you please create new issues for follow-up tasks or bugs?
Comment #158
joseph.olstadpatch 40 is for accessibility and has been in use with the panopoly distribution for quite some time
I believe that we should commit patch 40 for accessibility , just haven't gotten around to it yet.
soon.
Comment #159
joseph.olstadComment #160
joseph.olstadnevermind for patch 40. I was referring to another patch.
#2372907-2: Style selector not keyboard-accessible