Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Is AngularJS messing up with CSS :target selectors? #8478

Closed
binarykitchen opened this issue Aug 5, 2014 · 48 comments
Closed

Is AngularJS messing up with CSS :target selectors? #8478

binarykitchen opened this issue Aug 5, 2014 · 48 comments

Comments

@binarykitchen
Copy link

Looks like I cannot apply styles to :target selectors anymore. Is this because of AngularJS?

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

anymore

Which version of AngularJS did you upgrade to in noticing this issue? How are you trying to apply styles to :target selectors? What browsers are you seeing issues in? How are you determining that you aren't able to apply styles?

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

Lots of info is missing, if you could provide some details that would be helpful.

@binarykitchen
Copy link
Author

Ok, I am using AngularJS's npm package at https://www.npmjs.org/package/angular (current version is 1.2.19)

Then, here an example of the CSS I am trying:

ul#changelog li:target {
  background: #008000;
}

the html code is

<ul id="changelog" class="ng-scope">
    <li id="0.1.1">
        <h3 class="version"><a href="#0.1.1">Version 0.1.1</a>&nbsp;[current]</h3>
        <ul>
            <li>[feature] ...</li>
        </ul>
    </li>
    <li id="0.1.0">
        <h3 class="version"><a href="#0.1.0">Version 0.1.0</a></h3>
        <ul>
            <li>[feature] Revamp ...</li>
            <li>[feature] ...</li>
        </ul>
    </li>
</ul>

Fails on latest Firefox and Chrome. When I browse to https://localhost:8080/versions#0.1.1, the <li> tag with the id 0.1.1 should have the green background colour but doesn't.

Without AngularJS it works fine.

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

Seems to be working here http://plnkr.co/edit/E1dscGK7SO6SKtMnqt6C?p=preview

Please post a full reproduction (plnkr should work, but github is fine if you need that instead)

@binarykitchen
Copy link
Author

Thanks @caitp but you also should wrap the body with data-ng-view + data-ng-cloak, make it a single page app and enable the html5Mode to reproduce my case.

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

again, please post a full reproduction of your issue! package it! put it on the internets!

@binarykitchen
Copy link
Author

Well, it's a private repo ... in that case I could make you a collaborator so that you can git pull it. Would you do that?

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

you can squeeze it down to something that you can show publicly, that would be best.

@binarykitchen
Copy link
Author

ok, will set up something soon on my server, stay tuned

@jeffbcross
Copy link
Contributor

Hi @binarykitchen it'd be a lot more helpful if you could reduce the code to something that fits in a plunker, rather than requiring reviewing on your server.

@jeffbcross jeffbcross added this to the Purgatory milestone Aug 5, 2014
@binarykitchen
Copy link
Author

Okay @jeffbcross @caitp here is a reduced code on plunker:
http://plnkr.co/edit/N68dpnGCbl7eYsbdXnN8?p=preview

If you remove the <script async src="script.js"></script> part in the index.html then the CSS effect for li:target work. With the script, it doesn't.

The script itself is generated by browserify - on the top it's AngularJS, at the bottom you'll find the application-specific code. I removed anything obsolete to keep it as small as possible.

I hope this helps locating the bug!

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

It works as expected if you change the tags in your template to use target="_self", I suggest doing that

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

I believe I know the commit which caused this, and we are likely to revert it --- but when you expect the browser to do it's regular browser thing when you click links, you should use target="_self" anyways.

@caitp
Copy link
Contributor

caitp commented Aug 5, 2014

also, please try to avoid using requirejs for future reproductions, it's a lot easier to see what's going on in your app (and test against other versions of angular) if you just include your application code, and link to angular seperately.

@binarykitchen
Copy link
Author

Hmm, I added target="_self" on the <a> tag in my plunker but it still doesn't work.

Regarding reproduction, I agree. But if I remove browserify, then I'd have too many JS files to add.

Can you point me a link to the commit you believe caused this?

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

Did you add target="_self" to the <a> tag defined in your JS ($templateCache.put('whatever.html', '... all the markup from your actual index.html but defined in a template for whatever reason ...')` ?

Because that's what I had to do to make it work

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

Also, I think this might be fixed if we land #8492 --- it would be helpful if you tried a build of angular from that commit and see if that helps you

@binarykitchen
Copy link
Author

Oh right, I forgot the templateCache. Works now. http://plnkr.co/edit/N68dpnGCbl7eYsbdXnN8?p=preview

But there is a new problem:

  1. Click on Version 0.1.1 then it opens fine
  2. Then click on the other Version 0.1.0 and expect it to open asap. But it doesn't.
  3. Click again on Version 0.1.0 and then it opens

Looks like Angular is swallowing some events here. Look at https://videomail.io/versions#0.0.27 - this is the behaviour I want (currently without Angular).

(the templateCache is a trick to reduce the number of queries to the server. I am compiling the jade templates into that cache via Gulp)

@binarykitchen
Copy link
Author

and yes, if you email or post me an angular build with your commit i will test this for sure!

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

we might be swallowing the events anyways, but you can get it at http://ci.angularjs.org/job/angular.js-caitlin/456/artifact/build/

@binarykitchen
Copy link
Author

okay, tested with your builds. but the problem described in my comment above #8478 (comment) still exists.

i am afraid AngularJS is still messing up with these events ...

@binarykitchen
Copy link
Author

for example if i open the page for the first time with #0.1.1 in the URL, i want the CSS :target selector to become effective asap without being swalloed by AngularJS.

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

okay --- i'm not sure if this is really fixable at all in hashbang mode, but I think it will always work in html5 mode in browsers which support history?

At any rate it seems hard to write a non-e2e test for this one @_@

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

@binarykitchen you're welcome to come up with a patch for this btw, I think it comes down to just aborting if we determine the clicked href value is a "real" hash (IE not a LocationHashbangUrl virtual path) --- that part seems trivial, and it is related to a number of other issues open regarding $location (#772, #4608, sorta #5532, #5529, maybe others)

@binarykitchen
Copy link
Author

yeah, looks like these other issues are related to this one. i am afraid i am not that angular-experienced enough to patch angular for this. but i hope someone will in the near future.

many thanks for your assistance so far @caitp

@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

Okay, I'll see if I can write something for this.

caitp added a commit to caitp/angular.js that referenced this issue Aug 6, 2014
$location will no longer rewrite hash fragment links in the same document (specified via href="#hash...").

Closes angular#8478
@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

@binarykitchen you might try again using the build from http://ci.angularjs.org/job/angular.js-caitlin/457/artifact/build/

@binarykitchen
Copy link
Author

I tried and I am afraid, it is still not working with my prototype :(

@binarykitchen
Copy link
Author

Best is to copy the HTML code from https://videomail.io/versions#0.0.27 and trying to reproduce exactly the same behavior when AngularJS comes into play.

@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

http://plnkr.co/edit/WVPYzaXOKujcb5vSbvJz?p=preview seems to be working here

@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

aww I'm wrong, okay, looking into it. (it works if you don't inject $location, breaks if you do)

@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

okay, I think I know how to fix that :> (we do want to rewrite links which start with the hash prefix, like #!/foo/bar --- to avoid breaking hashbangmode sites --- but I guess the hashPrefix is the empty string by default, so a relative hash would always look the same as a hashPrefix path --- I've changed this by testing if it starts with '#' + hashPrefix + '/' instead)

caitp added a commit to caitp/angular.js that referenced this issue Aug 7, 2014
$location will no longer rewrite hash fragment links in the same document (specified via href="#hash...").

Closes angular#8478
@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

@binarykitchen
Copy link
Author

Yes, that's the behavior I want. But if you add HTML5 mode, it fails. See my plunker at
http://plnkr.co/edit/oh4z98EPV6BcaKmZBA6N?p=preview

Hope you can adjust the code further to make it work on HTML5 mode as well?

@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

Still works, there's just a syntax error in that updated plunk.

http://plnkr.co/edit/RKQUP0T5k0SnTq1xt2P3?p=preview

@binarykitchen
Copy link
Author

Okay, will add more code. Is there a way to see console errors in plunker?

@binarykitchen
Copy link
Author

Have extended your plunker with the router and not it doesn't work
http://plnkr.co/edit/fCZUnPO99cD2fWzIavfn?p=preview

You have to click twice on a link to make the CSS selector work.

@binarykitchen
Copy link
Author

Maybe the bug is related to the $routeProvider ?

@caitp
Copy link
Contributor

caitp commented Aug 7, 2014

There's a problem in $routeProvider which manifests in requiring the link to be clicked twice (with the patch), but that's a separate bug to be fixed separately imo

@binarykitchen
Copy link
Author

Ok! Should we open a new bug report for the $routeProvider in this tracker?

@binarykitchen
Copy link
Author

@caitp ??

@btford btford removed the gh: issue label Aug 20, 2014
@caitp
Copy link
Contributor

caitp commented Oct 1, 2014

@binarykitchen sorry I never got the message when you sent it. Yes, please file!

@caitp
Copy link
Contributor

caitp commented Oct 1, 2014

The patch I wrote for this didn't land, but it seems to be fixed by 2294880

@caitp caitp closed this as completed Oct 1, 2014
@binarykitchen
Copy link
Author

sorry, i cannot remember what to report here and where to. this ticket is pretty old ...

@caitp
Copy link
Contributor

caitp commented Oct 1, 2014

Have extended your plunker with the router and not it doesn't work
http://plnkr.co/edit/fCZUnPO99cD2fWzIavfn?p=preview

You have to click twice on a link to make the CSS selector work.

@binarykitchen
Copy link
Author

if your patch does fix this, then all should be good now

@caitp
Copy link
Contributor

caitp commented Oct 1, 2014

my patch had nothing whatsoever to do with the ngRoute issue, which is why I said you should file a bug about it! anyways, alright don't worry about it

@ripper2hl
Copy link

The :target selectors works fine finally?
i'm using building blocks for Firefox OS and the drawer style use the target selector :(

Check this out :
Drawer component
http://buildingfirefoxos.com/building-blocks/drawer.html

Drawer code
number line 13
https://github.com/buildingfirefoxos/Building-Blocks/blob/gh-pages/style/drawer.css

and this is my proyect(works fine with $anchorScroll and target="_self" but i think this not a very good solution)
http://www.proyectoclima.com/app/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants