Skip to content
This repository has been archived by the owner on Apr 21, 2021. It is now read-only.

Uses title HTML tag whenever is possible to update the page title. Closes #185 #186

Merged
merged 2 commits into from
Jan 6, 2017

Conversation

fernandosouza
Copy link
Contributor

No description provided.

@SpencerWoo
Copy link
Member

SpencerWoo commented Jan 6, 2017

@fernandosouza I don't believe this passes the tests of
brunobasto@7304323

or a test like

it('should handle html entities in setting the title', (done) => {
		var screen = new HtmlScreen();
		screen.load('/url').then(() => {
			assert.strictEqual('< > &', screen.getTitle());
			done();
		});
		this.requests[0].respond(200, null, '<title>< > &</title>');
	});

@@ -1106,7 +1106,14 @@ class App extends EventEmitter {
} else {
globals.window.history.pushState(state, title, path);
}
globals.document.title = title;

let titleNode = globals.document.querySelector('head title');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only document.querySelector('title'); is enough here

@fernandosouza
Copy link
Contributor Author

You are right @SpencerWoo. I'm going to fix the implementation. Thank you.

@fernandosouza
Copy link
Contributor Author

Pull updated. @eduardolundgren, please don't merge yet. I'm going to send another pull making Travis run the tests over Saucelabs for PRs.

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

Successfully merging this pull request may close these issues.

3 participants