Developer 827 #406

Merged
merged 2 commits into from Jul 24, 2014

Conversation

Projects
None yet
4 participants
Owner

pmuir commented Jul 21, 2014

@wesbos please review, as I suspect I broke something by adding the html tag to the page directly like this.

Contributor

jboss-developer-ci commented Jul 21, 2014

Triggering build using a merge of 0a6c566 on branch master:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/www.jboss.org-pull-player-executor/

Contributor

jboss-developer-ci commented Jul 21, 2014

Build 422 is now running using a merge of 0a6c566 on branch master:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/www.jboss.org-pull-player-executor/422

Contributor

jboss-developer-ci commented Jul 21, 2014

Related issue: DEVELOPER-827

@wesbos wesbos and 1 other commented on an outdated diff Jul 21, 2014

_layouts/base.html.slim
/[if IE 8]
| <html class="ie8 lt-ie9">
/[if IE 9]
| <html class="ie9">
-/ <!--[if (gte IE 9)|!(IE)]<!--> <html> <!--<![endif]-->
+/ [if (gte IE 9)|!(IE)]<!--> <html> <!--<![endif]-->
+html lang="en"
@wesbos

wesbos Jul 21, 2014

Contributor

Is there a reason you added it like this? Do you need to add the lang attribute? This will cause double HTML tags in all browsers

@pmuir

pmuir Jul 22, 2014

Owner

Yes, I need to add the lang attribute. What is the right way to do that?

@wesbos

wesbos Jul 22, 2014

Contributor

Yeah you'll need to put them in each HTML tag, like this:

pmuir#2

and then you also took out the comment on line 6, that is needed.

Owner

pmuir commented Jul 22, 2014

@wesbos unfortunately that doesn't seem to do anything - http://screencast.com/t/CRpVoY62C4lx

Contributor

wesbos commented Jul 22, 2014

@pmuir it added the html lang attr, isn't that what you wanted?

Owner

pmuir commented Jul 22, 2014

Yes, it’s what I want, but it didn’t add it.

On 22 Jul 2014, at 17:37, Wes Bos notifications@github.com wrote:

@pmuir it added the html lang attr, isn't that what you wanted?


Reply to this email directly or view it on GitHub.

Contributor

wesbos commented Jul 22, 2014

Ah I see, the last HTML tag doesn't get it added. Let me have a look

Contributor

wesbos commented Jul 22, 2014

Okay take a look at the PR I sent you, a little tricky with slim comments, to but it now ouputs the lang on all three html tags.

Contributor

jboss-developer-ci commented Jul 23, 2014

Triggering build using a merge of 9eced56 on branch master:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/www.jboss.org-pull-player-executor/

Contributor

jboss-developer-ci commented Jul 23, 2014

Build 453 is now running using a merge of 9eced56 on branch master:
Private: https://jenkins.mw.lab.eng.bos.redhat.com/hudson/job/www.jboss.org-pull-player-executor/453

Owner

pmuir commented Jul 23, 2014

Ok, this now looks good.

Contributor

paulrobinson commented Jul 24, 2014

@wesbos Are you now happy with this too?

Contributor

wesbos commented Jul 24, 2014

Yep

paulrobinson merged commit 9779a06 into jboss-developer:master Jul 24, 2014

1 check passed

jboss-developer-ci JBoss Developer CI build status: success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment