Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Fix issue where tab shows location instead of title (until mouseover)
Browse files Browse the repository at this point in the history
Also includes a fix for frame.js which caused a non-fatal error in the renderer process

Fixes #7765

Auditors: @cezaraugusto, @bbondy
  • Loading branch information
bsclifton committed Apr 12, 2017
1 parent da0b10f commit f07c142
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
2 changes: 1 addition & 1 deletion js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class Frame extends ImmutableComponent {
this.webview.send(messages.BRAVERY_DEFAULTS_UPDATED, this.props.braveryDefaults.toJS())
}
if (!Immutable.is(prevProps.extensions, this.props.extensions)) {
this.webview.send(messages.EXTENSIONS_UPDATED, this.props.extensions.toJS())
this.webview.send(messages.EXTENSIONS_UPDATED, this.props.extensions ? this.props.extensions.toJS() : null)
}
} else if (location === 'about:bookmarks' && this.props.bookmarks) {
if (!Immutable.is(prevProps.bookmarks, this.props.bookmarks) ||
Expand Down
21 changes: 18 additions & 3 deletions js/components/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,27 @@ class Tab extends ImmutableComponent {
} else if (this.props.tab.get('location') === 'about:newtab') {
return locale.translation('newTab')
}

// NOTE(bsclifton): this can't use a simple falsey check with fallback values
// since properties are involved. I believe the title is populated page is loaded
// (which means it uses location first). When title is truthy, React gets confused
// and renders the location (but shows the title in the DOM). This may be a bug in React.
//
// Here's a demo of the syntax which causes the problem:
// return this.props.tab.get('title') || this.props.tab.get('location') || ''

const title = this.props.tab && this.props.tab.get('title')
const location = this.props.tab && this.props.tab.get('location')
const display = typeof title === 'undefined'
? typeof location === 'undefined'
? ''
: location
: title

// YouTube tries to change the title to add a play icon when
// there is audio. Since we have our own audio indicator we get
// rid of it.
return (this.props.tab.get('title') ||
this.props.tab.get('location') ||
'').replace('▶ ', '')
return display.replace('▶ ', '')
}

onDragStart (e) {
Expand Down

2 comments on commit f07c142

@bbondy
Copy link
Member

@bbondy bbondy commented on f07c142 Apr 13, 2017

Choose a reason for hiding this comment

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

I'm not following the explanation but as long as it works I'm happy

@bsclifton
Copy link
Member Author

Choose a reason for hiding this comment

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

🦂

Please sign in to comment.