Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
- In Firefox and IE with enabled hardware acceleration (and supporting Subpixel), the input fields are rendered sightly taller. This causes part or all of the spinning/searching state of the throbber in autocomplete fields to be revealed - even while the autocomplete is in idle state (see screenshot above).
- Furthermore, if the input font size is increased or if extra padding is added to inputs, styles are also broken as above shown - in all browsers.
- In some browsers, like old Chrome (#1891764: Autocomplete throbber causes excessive CPU usage on some browsers) animated GIFs cause excessive CPU usage. Having the animated searching/spinning state and the idle state of the throbber in a single animated GIF file forces us to load it in all pages that have an autocomplete field. So the CPU issue occurs even when the autocomplete is idle. What's worse is that most of the time users have no clue why their browser hogs their system only on some sites (made with Drupal) and only in certain pages (that contain autocomplete fields).
Proposed resolution
- Split throbber.gif into 2 files and hide the animated/spinning GIF file by default to resolve high CPU usage.
- Remove the fixed value and let browsers vertical align the image correctly.
Remaining tasks
N/A
User interface changes
On buggy browsers, it shows the throbber correctly:
API changes
N/A
Original report by @ogi
When Bartik is set for adding and editing content, throbber in textfield shows part of the rotating throbber. See the attached screenshot.
I fixed this by changing background-position
in my Bartik subtheme by adding 4px
. Additionally, I moved throbber slightly left. The second attachment shows the result.
/* Animated throbber */
html.js input.form-autocomplete {
background-image: url(../../../../../misc/throbber.gif);
background-position: 99% 6px; /* LTR */
background-repeat: no-repeat;
}
html.js input.throbbing {
background-position: 99% -14px; /* LTR */
}
Comment | File | Size | Author |
---|---|---|---|
#114 | after.png | 8.87 KB | David_Rothstein |
#114 | before.png | 8.88 KB | David_Rothstein |
#114 | 1069152-114-followup.patch | 483 bytes | David_Rothstein |
#108 | 1069152-throbber-108.patch | 5.78 KB | Mixologic |
#82 | after-throbbing.png | 11.23 KB | Steven Jones |
Comments
Comment #1
droplet CreditAttribution: droplet commentedtagging IE
Comment #2
ogi CreditAttribution: ogi commentedIt's not IE-only, I observe this behaviour in Firefox 4 Beta under Windows 7.
Unfortunately my changes are not enough. Attached is how Tags field is shown with my changes.
Comment #3
droplet CreditAttribution: droplet commentedoh cool, forgot Im using IE9
I do some test and it seems only happen on browser with hardware acceleration enabled .
enabled hardware acceleration, inputs height will increase... FTW!! (related to font-size: 0.929em, Sub-pixel problem)
Comment #4
ogi CreditAttribution: ogi commentedYes, after setting
gfx.direct2d.disabled
totrue
throbber looks OK.Comment #5
klonosI see this too in latest dev from March 10. I only came across it in the 'Authored by' field in node create/edit. Any progress?
Comment #6
droplet CreditAttribution: droplet commentedlimited height or remove INPUTs padding is a way to solve this problem
Comment #7
droplet CreditAttribution: droplet commentedI edited throbber.gif to have more space between hover & non-hover state. and adding JS to calc elements height and apply right background position.
It's potential risk to other themes as well.
Comment #9
klonos@droplet: can you please attach the .gif as a file instead of a binary diff please.
Comment #10
droplet CreditAttribution: droplet commented@klonos,
sure
correct my comment on #7
JS is calc elements inner height (height + padding). because padding is one of the main reason cause this problem.
Comment #11
klonos...thanx, this fixes things, but I don't actually know how it will do with themes other then Bartik.
Comment #12
droplet CreditAttribution: droplet commentedComment #13
marcoka CreditAttribution: marcoka commented#10 would help fixing all issues regarding to the change of height of an auto complete input field. meaning you can alter the css and fix it. currently you have to replace the throbber.
Comment #14
jhedstromHere's #10 rerolled to fix some whitespace and to apply against 8.x. Most of the changes appear to have already been committed.
Comment #15
webchickOh, nice! This has been driving me nuts in the Spark/Edit queue.
I can't actually have the front-end chops to give this a final RTBC but very excited to see it being worked on! :D
Futzing with some metadata.
Comment #16
ry5n CreditAttribution: ry5n commentedSo the problem here is the classic one where a sprite will have adjacent elements bleed into view if the size of the container exceeds the sprite spacing in the image. Although it looks like the proposed solution would work (extra space in the sprite), using JS to set the background position seems… like not the right tool for the job.
A better approach in my experience is to use dedicated markup to contain the icon, something like:
.
The icon container would have 'width' and 'height' declared so the sprite spacing is guaranteed not to be an issue. It can also be positioned correctly with just CSS. This is another issue that could benefit from http://drupal.org/node/1849712.
Comment #17
droplet CreditAttribution: droplet commentedSigh! 2 years old patch, no one review it.
@ryn5,
I was patched for D7 ( no markup changes), :)
Comment #18
ry5n CreditAttribution: ry5n commented@droplet I don’t understand #17. Can you elaborate?
Comment #19
relaxpor CreditAttribution: relaxpor commentedoverride the css file with these value
For an idea, in the corolla subtheme.
This is an example to display no throbber when no load
and display a mythrobber.gif when there is load
html.js input.form-autocomplete {
background-image: none;
}
html.js input.throbbing {
background-image: url(../../../../misc/mythrobber.gif);
background-position: 98% 10px;
}
Comment #20
quicksketchThis is also what we're doing in system.base.css for the AJAX throbber CSS. +1 to r5yn's approach.
Comment #21
Uccio CreditAttribution: Uccio commentedI can confirm that #14 javascript patch fix the problem on D7 (Bartik 7.22 and Garland 7.22).
My working solution uses the image contained in #10 because I have met some difficulties to patch the image file.
Comment #22
almc CreditAttribution: almc commentedI've got a message for the patch #14: '... cannot be applied in the selected context', tried it with Drupal 7.23. The issue is showing up as a partially visible continuously spinning throbber, even when the autocomplete field is empty. This happens for D7.23 Bartik theme, in IE and Firefox particularly, wile looking good in Opera.
Comment #23
almc CreditAttribution: almc commented14: throbber-alignment-1069152-14.patch queued for re-testing.
Comment #25
almc CreditAttribution: almc commentedThe patch #10 (.gif replacement) fixes the issue (no continuous spinning), however:
Comment #26
alexandrezia CreditAttribution: alexandrezia commentedThis is the same patch from comment #14
Just fixing file paths so the patch can be applied by drush make
Comment #27
alexandrezia CreditAttribution: alexandrezia commentedComment #29
alexandrezia CreditAttribution: alexandrezia commentedComment #30
alexandrezia CreditAttribution: alexandrezia commentedComment #31
alexandrezia CreditAttribution: alexandrezia commented26: throbber-alignment-1069152-14-2.patch queued for re-testing.
Comment #33
alexandrezia CreditAttribution: alexandrezia commentedSetting to 7.x branch
Comment #34
alexandrezia CreditAttribution: alexandrezia commentedComment #35
gmclelland CreditAttribution: gmclelland commentedI think we would be better off splitting the gif into two separate files and using just CSS to position the throbbers.
The reason I say two files is because it is an animated sprite that is always animating even if you can't visible see it.
To see the problem, turn on painting using the Google Chrome Development Tools or Firefox's Development Tools and you will see the throbber is constantly being repainted.
To eliminate the extra http request maybe we can inline the two small files in the css? See http://css-tricks.com/data-uris/
What does everyone think?
Comment #36
gmclelland CreditAttribution: gmclelland commentedComment #37
gmclelland CreditAttribution: gmclelland commentedComment #38
droplet CreditAttribution: droplet commented- Two separate image files
- Fixed GFX issue (padding issue)
- Fixed CPU issue
Comment #40
droplet CreditAttribution: droplet commented38: 1069152-throbber.patch queued for re-testing.
Comment #42
droplet CreditAttribution: droplet commentedTestbots error ?
Comment #43
droplet CreditAttribution: droplet commentedComment #46
gmclelland CreditAttribution: gmclelland commented@droplet - how about using a base64 encoded file in the css? That would eliminate the extra http requests for each throbber. Normally I use http://compass-style.org to do this.
Here is an example of what I'm talking about http://keegan.st/2012/09/14/easy-image-data-uris-with-compass/
Comment #47
droplet CreditAttribution: droplet commentedPersonally, I don't think we have to include 500 bytes (or around 3kb of 2 images) extra data to every site. And there's not used for any modules in D8 CORE. I will keep as it for now. :)
It seems like the testbot has problems at the moment, I will retest the patch soon.
Comment #48
droplet CreditAttribution: droplet commented38: 1069152-throbber.patch queued for re-testing.
Comment #50
droplet CreditAttribution: droplet commentedReroll
Comment #51
droplet CreditAttribution: droplet commentedComment #52
almc CreditAttribution: almc commented50: 1069152-throbber-50.patch queued for re-testing.
Comment #53
almc CreditAttribution: almc commentedTriggered re-testing, the patch couldn't be opened in an editor.
Comment #55
droplet CreditAttribution: droplet commentedSame as Patch #55 and updated to jQuery UI coding.
- Fixed new jQuery UI Autocomplete alignment problem
Comment #56
almc CreditAttribution: almc commented55: 1069152-throbber-55.patch queued for re-testing.
Comment #57
almc CreditAttribution: almc commentedTriggered re-testing, I applied the patch to the folder /misc, however haven't noticed a difference in throbber appearance in autocomplete field.
Comment #58
droplet CreditAttribution: droplet commentedin D8, you have to clear caches after any styles changes.
Comment #59
almc CreditAttribution: almc commentedI thought the patch is for D7 as D8 is not ready for production use yet, and the problem does exist for D7.
Comment #60
droplet CreditAttribution: droplet commentedThis is Patch for D8 :)
I will add D7 Patch soon.
Please review it :)
Comment #61
droplet CreditAttribution: droplet commented55: 1069152-throbber-55.patch queued for re-testing.
Comment #62
droplet CreditAttribution: droplet commented55: 1069152-throbber-55.patch queued for re-testing.
Comment #63
droplet CreditAttribution: droplet commented55: 1069152-throbber-55.patch queued for re-testing.
Comment #64
eigentor CreditAttribution: eigentor commentedTested the patch with latest dev and works.
Maybe you should write a reasoning what you did and why with your patch.
As far as I can see, it divides the throbber into two images instead of one, so the image is always perfectly placed, even if the input field gets big:
Comment #65
klonosThis looks great! Can't wait to see make its way back to D7.
Comment #66
almc CreditAttribution: almc commentedagree, given that D7 version is the production platform of choice now ...
Comment #67
droplet CreditAttribution: droplet commentedComment #68
klonosComment #69
eigentor CreditAttribution: eigentor commentedTo RTBC this i guess it needs a code review. It being mostly in system.module.css and not in sevens css files seems right to me, but I don't know if there are any special policies as to which css files to use.'
The css itself cannot be much wrong: it is set to right and vertically centered. Not much more in here. So I'll be bold and set it RTBC, maybe this will trigger someone else to put it under scrutiny.
Comment #70
sunThis is a great clean-up and we should move forward here.
That said, I see two issues that could be addressed in follow-ups:
The throbber looks a bit squeezed to the right border.
→ Instead of 100%, we could consider to use 98% (or a fancy CSS trick, if there is any).
I wonder why this is an animated GIF in the first place?
→ If we'd consider to put the throbber next to the input element (instead of inside), then CSS3 is simple as this:
cf. http://jsfiddle.net/uv8a5/
See also:
http://37signals.com/svn/posts/2577-loading-spinner-animation-using-css-...
http://aino.github.io/throbber.js/
http://onwebdev.blogspot.com/2011/01/css3-background-rotate-property.html
http://blog.freeside.co/post/42357804660/css-background-transformations
Comment #71
klonos@sun: Great idea about using CSS animation instead! Filed #2181399: Consider moving the animated .GIF throbber outside the autocomplete fields and using CSS animation instead. for your suggestion (and gave credits to you for it).
Comment #72
alexpottNice work! Committed 8910339 and pushed to 8.x. Thanks!
Comment #73
alexpottComment #74
cmonnow CreditAttribution: cmonnow commentedInterestingly, I only noticed on my PC a few weeks ago that drupal.org's animated throbber was peeking through at the bottom of inputs in all of my browsers (in Ubuntu 12.04), which I never noticed previously. I wasn't sure if it was a site change or some visual configuration was changed on my setup.
Comment #75
klonosYep, still an issue and now that d.o runs on D7 it hits more users:
Comment #76
droplet CreditAttribution: droplet commentedComment #77
nitishchopra CreditAttribution: nitishchopra commentedThanks for that patch. This is how the throbber field looks like on firefox 28 on ubantu 12.04.The issue seems to be solved.
Comment #78
tstoecklerPer #75
Comment #79
Manjit.Singh76: 1069152-throbber.patch queued for re-testing.
Comment #80
mgifford#77 seemed like a good review, even though it was on April Fools.
@nitishchopra any reason you didn't mark it RTBC? Also, just to confirm, you've got hardware acceleration on and see the bug without the patch?
Comment #81
droplet CreditAttribution: droplet commentedNotes to reviewers,
You won't have to enabled / disabled hardware acceleration on your browsers. You can apply Padding to input box. It's same results. Some tests already preformed on D8 patch. Generally said, I think it only needed code reviews.
Thanks.
Comment #82
Steven Jones CreditAttribution: Steven Jones commentedBefore inactive:
Before throbbing:
After inactive:
After throbbing:
Patch looks okay technically, but I think this constitutes and kind of an API change doesn't it?
In the patch Bartik and Seven need to be adjusted to make the new throbber work, and any themes that assume the structure and do similar things, will need to be changed too? This could cause a lot of annoyance for themes. Although I admit that when I've done this in themes, I've basically removed all of the basic throbber stuff, and replaced it with my own.
This should be called out in the release notes at the very least.
I'm happy to mark to this as RTBC in the near future if someone is happy that the 'API change' is okay.
Comment #83
droplet CreditAttribution: droplet commentedthe code on themes just overrided the CORE base styles. On the patch above, we removing the excessive code from Bartik & Seven only. If you have similar code on your theme. It remains SAME results. (That means, no fixes to your themes)
I won't consider it's an API change, at least it's not a edge cases.
Comment #84
Steven Jones CreditAttribution: Steven Jones commentedRTBC then, because of #82 and #83
Comment #86
droplet CreditAttribution: droplet commented76: 1069152-throbber.patch queued for re-testing.
Comment #87
mgifford76: 1069152-throbber.patch queued for re-testing.
Comment #88
mgiffordSorry, @droplet already did that. It passes so back to RTBC.
Comment #91
David_Rothstein CreditAttribution: David_Rothstein commentedTestbot failure.
Comment #94
dcam CreditAttribution: dcam commentedComment #97
dcam CreditAttribution: dcam commentedComment #100
dcam CreditAttribution: dcam commentedComment #102
droplet CreditAttribution: droplet commentedComment #106
mgiffordbad bot.
Comment #108
MixologicReuploading a patch with non-duplicate file names.
Comment #109
MixologicComment #110
jthorson CreditAttribution: jthorson commentedRetest passed.
Comment #111
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks!
I deliberately did not delete misc/throbber.gif during the commit though (we should leave that in place since other CSS out there certainly might still be referencing it).
Also:
I'm pretty sure we need a small followup to fix this in Bartik for RTL languages also?
Comment #113
droplet CreditAttribution: droplet commentedNo. RTL is already done on base CSS.
Comment #114
David_Rothstein CreditAttribution: David_Rothstein commentedYes, but Bartik overrides that.
See the attached patch and screenshots. It's not a huge difference, but before the patch it still looks a bit misaligned to me, and after it looks fixed.
Before:
After:
Comment #115
joelpittetGood catch thanks @David_Rothstein, this is consistent with the rest of the patch.
Comment #116
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks!