Opened on 02/05/2015 at 01:30:58 PM
Closed on 02/01/2016 at 12:55:14 PM
Last modified on 02/24/2016 at 11:08:28 AM
#1950 closed defect (fixed)
Tooltip not displayed for very long filters
| Reported by: | mapx | Assignee: | greiner |
|---|---|---|---|
| Priority: | P4 | Milestone: | Adblock-Plus-2.7.2-for-Firefox |
| Module: | Adblock-Plus-for-Firefox | Keywords: | goodfirstbug |
| Cc: | trev, arthur, greiner | Blocked By: | |
| Blocking: | Platform: | Firefox | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by trev)
Environment
firefox 36
ABP 2.6.7.3895
easylist, easyprivacy
How to reproduce
-go to (an example)
http://www.caranddriver.com/photo-gallery/2017-ford-focus-rs-official-photos-and-info-news
-open blockable items list
-in the list there is a hidden element ##.advertisement
-hover the mouse above the filter (to see in which list is the filer)
-nothing happens
-the same behaviour is when you search in easylist the filter and then hover above the filter
-the filter is very long, so probably it's some limit ...
1520wbzw.com,760kgu.biz,880thebiz.com,about.com,afro.com,allmusic.com,amctv.com,animax-asia.com,ap.org,araratadvertiser.com.au,areanews.com.au,armidaleexpress.com.au,avclub.com,avonadvocate.com.au,axn-asia.com,barossaherald.com.au,batemansbaypost.com.au,baysidebulletin.com.au,begadistrictnews.com.au,bellingencourier.com.au,bendigoadvertiser.com.au,betvasia.com,bigthink.com,biz1190.com,bizarremag.com,blacktownsun.com.au,blayneychronicle.com.au,bluemountainsgazette.com.au,boingboing.net,bombalatimes.com.au,boorowanewsonline.com.au,bordermail.com.au,braidwoodtimes.com.au,bravotv.com,brimbankweekly.com.au,bunburymail.com.au,business1110ktek.com,business1570.com,businessinsurance.com,busseltonmail.com.au,camdenadvertiser.com.au,camdencourier.com.au,canowindranews.com.au,caranddriver.com,carrierethernetnews.com,caseyweekly.com.au,caseyweeklycranbourne.com.au,centraladvocate.com.au,centralwesterndaily.com.au,cessnockadvertiser.com.au,cinemablend.com,classicandperformancecar.com,clickhole.com,colliemail.com.au,colypointobserver.com.au,competitor.com,coomaexpress.com.au,cootamundraherald.com.au,cowraguardian.com.au,crainsnewyork.com,crookwellgazette.com.au,crosswalk.com,dailyadvertiser.com.au,dailygazette.com,dailyliberal.com.au,dailyrecord.com,dandenongjournal.com.au,defenceweb.co.za,di.fm,donnybrookmail.com.au,downloadcrew.com,dunedintv.co.nz,dungogchronicle.com.au,easternriverinachronicle.com.au,edenmagnet.com.au,elliottmidnews.com.au,esperanceexpress.com.au,essentialmums.co.nz,examiner.com.au,eyretribune.com.au,fairfieldchampion.com.au,fastcocreate.com,fastcodesign.com,financialcontent.com,finnbay.com,forbesadvocate.com.au,frankstonweekly.com.au,gazettextra.com,gematsu.com,gemtvasia.com,gippslandtimes.com.au,gleninnesexaminer.com.au,globest.com,gloucesteradvocate.com.au,goodcast.org,goondiwindiargus.com.au,goulburnpost.com.au,greatlakesadvocate.com.au,grenfellrecord.com.au,guyraargus.com.au,hardenexpress.com.au,hawkesburygazette.com.au,hepburnadvocate.com.au,hillsnews.com.au,hispanicbusiness.com,humeweekly.com.au,huntervalleynews.net.au,i-dressup.com,imgur.com,inverelltimes.com.au,irishtimes.com,juneesoutherncross.com.au,kansas.com,katherinetimes.com.au,kdow.biz,kkol.com,knoxweekly.com.au,lakesmail.com.au,lamag.com,latrobevalleyexpress.com.au,legion.org,lithgowmercury.com.au,liverpoolchampion.com.au,livestrong.com,livetennis.com,macarthuradvertiser.com.au,macedonrangesweekly.com.au,macleayargus.com.au,magtheweekly.com,mailtimes.com.au,maitlandmercury.com.au,mandurahmail.com.au,manningrivertimes.com.au,margaretrivermail.com.au,maribyrnongweekly.com.au,marinmagazine.com,marketwatch.com,maroondahweekly.com.au,meltonweekly.com.au,merimbulanewsonline.com.au,merredinmercury.com.au,metservice.com,monashweekly.com.au,money1055.com,mooneevalleyweekly.com.au,moreechampion.com.au,movies4men.co.uk,mprnews.org,msn.com,mudgeeguardian.com.au,murrayvalleystandard.com.au,muswellbrookchronicle.com.au,myallcoastnota.com.au,nambuccaguardian.com.au,naroomanewsonline.com.au,narrominenewsonline.com.au,nationalgeographic.com,newcastlestar.com.au,northernargus.com.au,northerndailyleader.com.au,northweststar.com.au,nvi.com.au,nynganobserver.com.au,nytimes.com,oann.com,oberonreview.com.au,onetvasia.com,onlinegardenroute.co.za,orange.co.uk,parenthood.com,parkeschampionpost.com.au,parramattasun.com.au,pch.com,peninsulaweekly.com.au,penrithstar.com.au,plasticsnews.com,portlincolntimes.com.au,portnews.com.au,portpirierecorder.com.au,portstephensexaminer.com.au,praguepost.com,psychologytoday.com,queanbeyanage.com.au,racingbase.com,radioguide.fm,redsharknews.com,rhsgnews.com.au,riverinaleader.com.au,roxbydownssun.com.au,rubbernews.com,saitnews.co.za,sconeadvocate.com.au,silverdoctors.com,singletonargus.com.au,smallbusiness.co.uk,sonychannel.co.za,sonychannelasia.com,sonymax.co.za,sonymoviechannel.co.uk,sonytv.com,southcoastregister.com.au,southernhighlandnews.com.au,southernweekly.com.au,southwestadvertiser.com.au,standard.net.au,star-telegram.com,stawelltimes.com.au,stmarysstar.com.au,stonningtonreviewlocal.com.au,summitsun.com.au,suncitynews.com.au,sunjournal.com,sunraysiadaily.com.au,tennantcreektimes.com.au,tenterfieldstar.com.au,theadvocate.com,theadvocate.com.au,thebeachchannel.tv,thecourier.com.au,thecurrent.org,theflindersnews.com.au,theforecaster.net,theguardian.com.au,theherald.com.au,theislanderonline.com.au,theleader.com.au,thenortherntimes.com.au,theridgenews.com.au,therural.com.au,tirebusiness.com,townandcountrymagazine.com.au,transcontinental.com.au,travelpulse.com,twitch.tv,ulladullatimes.com.au,victorharbortimes.com.au,waginargus.com.au,walchanewsonline.com.au,walworthcountytoday.com,washingtonexaminer.com,wauchopegazette.com.au,wellingtontimes.com.au,westcoastsentinel.com.au,westernadvocate.com.au,westernmagazine.com.au,whyallanewsonline.com.au,winghamchronicle.com.au,wollondillyadvertiser.com.au,woot.com,wsj.com,wyndhamweekly.com.au,yasstribune.com.au,yellowpages.ca,youngwitness.com.au##.advertisement
What to change
Blockable items tooltip (hovering any row):
For any filter longer than 482 characters, only show the first and last 240 characters. Replace the middle of the filter by an ellipsis ("\u2026"). Same logic should be applied to the address row as well.
Filter preferences tools (hovering a filter):
Apply the logic outlined above to the filter row.
Attachments (0)
Change History (15)
comment:1 Changed on 02/05/2015 at 01:31:39 PM by mapx
- Cc arthur added
comment:3 Changed on 02/12/2015 at 03:42:39 PM by greiner
- Cc greiner added
comment:4 Changed on 02/12/2015 at 03:53:44 PM by mapx
it might be a good compromise
comment:5 in reply to: ↑ 2 ; follow-up: ↓ 8 Changed on 02/12/2015 at 07:01:24 PM by trev
- Description modified (diff)
- Keywords goodfirstbug added
- Priority changed from Unknown to P4
- Ready set
Unfortunately, I'm unaware of any existing mechanisms that would allow truncating a multiline string. So it does sound like we have to implement our own mechanism.
Replying to greiner:
I'd suggest truncating long filters (filters with more than 400 characters)
...
This approach would show the first and last 320 characters of the filter
So we will truncate filters longer than 400 characters but still happily show 640 characters? This makes no sense to me. Let's rather agree to show the first and last 200 characters of the filter - for anything longer than 402 characters (not for 401 characters because there we would merely replace one character by ellipsis which makes no sense to save space).
comment:7 Changed on 02/12/2015 at 07:05:23 PM by trev
- Summary changed from very long filter - viewing problem to Tooltip not displayed for very long filters
comment:8 in reply to: ↑ 5 Changed on 02/12/2015 at 08:11:43 PM by greiner
Replying to trev:
Unfortunately, I'm unaware of any existing mechanisms that would allow truncating a multiline string. So it does sound like we have to implement our own mechanism.
We are already creating 80-character long substrings from that string so it shouldn't produce too much overhead as long as we're sticking with multiples of 80.
Replying to greiner:
I'd suggest truncating long filters (filters with more than 400 characters)
...
This approach would show the first and last 320 characters of the filter
So we will truncate filters longer than 400 characters but still happily show 640 characters? This makes no sense to me. Let's rather agree to show the first and last 200 characters of the filter - for anything longer than 402 characters (not for 401 characters because there we would merely replace one character by ellipsis which makes no sense to save space).
I chose 320 since it was a multiple of 80. Admittingly, 400 (5 lines worth of characters) was incorrect… I actually meant 720 (9 lines worth of characters) since this is the maximum lines I imagined we'd want to occupy (as seen in the example). Therefore everything exceeding 720 characters would make sense to be truncated since it would otherwise introduce a tenth line.
comment:9 Changed on 02/12/2015 at 08:22:59 PM by trev
- Description modified (diff)
Right, let's go with multiples of 80 then - I changed the numbers into 240 and 482. I think that seven lines of text should really be enough.
comment:10 Changed on 05/20/2015 at 02:22:39 PM by philll
- Platform changed from Firefox/Firefox Mobile to Firefox
Made Firefox and Firefox mobile available as seperate platforms.
comment:11 Changed on 01/26/2016 at 02:22:24 PM by greiner
- Owner set to greiner
- Tester set to Unknown
- Verified working unset
comment:12 Changed on 01/26/2016 at 04:17:44 PM by greiner
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:13 Changed on 02/01/2016 at 12:55:14 PM by greiner
- Resolution set to fixed
- Status changed from reviewing to closed
comment:14 Changed on 02/01/2016 at 12:55:28 PM by greiner
- Milestone set to Adblock-Plus-for-Firefox-next
comment:15 Changed on 02/24/2016 at 11:08:28 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Row width limit and ellipse working in both E10S and non-E10S Firefox.
ABP 2.7.2
Firefox 38.0 / Firefox 46.0a1 (Nightly) / Ubuntu 14.04 x64
Firefox 38.0 / Firefox 43.0.1 / Firefox 46.0a1 (Nightly) / Windows 7 x64
The issue is that this filter would, with the current implementation, require 63 lines to be added to the tooltip which, much likely, would make the tooltip grow too large anyway. Since it wouldn't make much sense to have a scrollable box inside of a tooltip, I'd suggest truncating long filters (filters with more than 400 characters) and show them in the following form:
This approach would show the first and last 320 characters of the filter based on the assumption that the middle bit is negligible for display in the tooltip. Would that be sufficient?