Merge pull request #8603 from brave/title-update

update title on changes

app/renderer/components/navigation/navigationBar.js

@@ -112,11 +112,12 @@ class NavigationBar extends React.Component {
   }
 
   get titleMode () {
+    const hasTitle = this.props.title && this.props.location && this.props.title !== this.props.location.replace(/^https?:\/\//, '')
     return this.props.activeTabShowingMessageBox ||
       (
         this.props.mouseInTitlebar === false &&
         !this.props.bookmarkDetail &&
-        this.props.title &&
+        hasTitle &&
         !['about:blank', 'about:newtab'].includes(this.props.location) &&
         !this.loading &&
         !this.props.navbar.getIn(['urlbar', 'focused']) &&

docs/windowActions.md

@@ -85,15 +85,13 @@ Dispatches a message when the guestInstanceId changes for a frame
 
 
 
-### tabDataChanged(frameProps, tabData) 
+### tabDataChanged(tabs) 
 
 Dispatches a message when tab data changes
 
 **Parameters**
 
-**frameProps**: `Object`, The frame properties
-
-**tabData**: `Object`, the tab properties
+**tabs**: `Object`, the tab properties
 
 
 

js/actions/windowActions.js

@@ -103,14 +103,12 @@ const windowActions = {
 
   /**
    * Dispatches a message when tab data changes
-   * @param {Object} frameProps - The frame properties
-   * @param {Object} tabData - the tab properties
+   * @param {Object} tabs - the tab properties
    */
-  tabDataChanged: function (frameProps, tabData) {
+  tabDataChanged: function (tabs) {
     dispatch({
       actionType: windowConstants.WINDOW_TAB_DATA_CHANGED,
-      frameProps,
-      tabData
+      tabs
     })
   },
 

js/components/frame.js

@@ -52,7 +52,6 @@ class Frame extends ImmutableComponent {
     this.onCloseFrame = this.onCloseFrame.bind(this)
     this.onUpdateWheelZoom = debounce(this.onUpdateWheelZoom.bind(this), 20)
     this.onFocus = this.onFocus.bind(this)
-    this.onAppStateChange = this.onAppStateChange.bind(this)
     // Maps notification message to its callback
     this.notificationCallbacks = {}
     // Counter for detecting PDF URL redirect loops
@@ -148,7 +147,6 @@ class Frame extends ImmutableComponent {
   }
 
   componentWillUnmount () {
-    appStoreRenderer.removeChangeListener(this.onAppStateChange)
     this.expireContentSettings(this.origin)
   }
 
@@ -210,15 +208,7 @@ class Frame extends ImmutableComponent {
     }
   }
 
-  onAppStateChange () {
-    if (!this.frame.isEmpty() && this.tab && !this.tab.delete('frame').equals(this.lastTab)) {
-      windowActions.tabDataChanged(this.frame, this.tab)
-    }
-    this.lastTab = this.tab && this.tab.delete('frame')
-  }
-
   componentDidMount () {
-    appStoreRenderer.addChangeListener(this.onAppStateChange)
     if (this.props.isActive) {
       windowActions.setActiveFrame(this.frame)
     }

js/stores/windowStore.js

@@ -6,6 +6,7 @@ const AppDispatcher = require('../dispatcher/appDispatcher')
 const EventEmitter = require('events').EventEmitter
 const appActions = require('../actions/appActions')
 const appConstants = require('../constants/appConstants')
+const windowActions = require('../actions/windowActions')
 const windowConstants = require('../constants/windowConstants')
 const config = require('../constants/config')
 const settings = require('../constants/settings')
@@ -25,7 +26,11 @@ const {aboutUrls, getTargetAboutUrl, newFrameUrl} = require('../lib/appUrlUtil')
 const Serializer = require('../dispatcher/serializer')
 const {updateTabPageIndex} = require('../../app/renderer/lib/tabUtil')
 const assert = require('assert')
-const contextMenuState = require('../../app/common/state/contextMenuState.js')
+const contextMenuState = require('../../app/common/state/contextMenuState')
+const tabState = require('../../app/common/state/tabState')
+const appStoreRenderer = require('./appStoreRenderer')
+
+let previousTabs = new Immutable.List()
 
 let windowState = Immutable.fromJS({
   activeFrameKey: null,
@@ -225,13 +230,16 @@ const frameGuestInstanceIdChanged = (state, action) => {
 
 const tabDataChanged = (state, action) => {
   action = makeImmutable(action)
-  const tabData = action.get('tabData')
-  const frameProps = action.get('frameProps')
-
-  const newProps = frameStateUtil.getFramePropsFromTab(tabData)
-  state = state.mergeIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps)
-  state = state.mergeIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps)
-
+  const tabs = action.get('tabs')
+
+  tabs.forEach((tab) => {
+    const newProps = frameStateUtil.getFramePropsFromTab(tab)
+    const frameProps = frameStateUtil.getFrameByTabId(state, tab.get('tabId'))
+    if (frameProps) {
+      state = state.mergeIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps)
+      state = state.mergeIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps)
+    }
+  })
   return state
 }
 
@@ -818,6 +826,20 @@ ipc.on(messages.DISPATCH_ACTION, (e, serializedPayload) => {
   }
 })
 
+const onAppStateChange = () => {
+  setImmediate(() => {
+    const tabs = tabState.getTabs(appStoreRenderer.state).map((tab) => {
+      return tab.delete('frame')
+    })
+    if (tabs.hashCode() !== previousTabs.hashCode()) {
+      previousTabs = tabs
+      windowActions.tabDataChanged(tabs)
+    }
+  })
+}
+
+appStoreRenderer.addChangeListener(onAppStateChange)
+
 AppDispatcher.register(doAction)
 
 module.exports = windowStore

test/bookmark-components/bookmarksTest.js

@@ -172,6 +172,7 @@ describe('bookmark tests', function () {
 
       before(function * () {
         this.pageNoTitle = Brave.server.url('page_no_title.html')
+        this.title = this.pageNoTitle.replace(/http:\/\//, '')
 
         yield setup(this.app.client)
 
@@ -181,48 +182,23 @@ describe('bookmark tests', function () {
           .windowParentByUrl(this.pageNoTitle)
           .activateURLMode()
           .waitForExist(navigatorNotBookmarked)
-          .activateURLMode()
           .click(navigatorNotBookmarked)
-          .waitForBookmarkDetail(this.pageNoTitle, '')
+          .waitForBookmarkDetail(this.pageNoTitle, this.title)
           .waitForEnabled(doneButton + ':not([disabled]')
       })
 
-      it('leaves the title field blank', function * () {
+      it('sets the title to the url', function * () {
         yield this.app.client
           .waitForExist(bookmarkNameInput)
-          .waitForInputText(bookmarkNameInput, '')
+          .waitForInputText(bookmarkNameInput, this.title)
       })
 
       it('does not show the url field', function * () {
         yield this.app.client
           .waitForElementCount(bookmarkLocationInput, 0)
       })
-
-      describe('saved without a title', function () {
-        before(function * () {
-          yield this.app.client
-            .waitForBookmarkDetail(this.pageNoTitle, '')
-            .waitForEnabled(doneButton)
-            .click(doneButton)
-        })
-        it('displays URL', function * () {
-          yield this.app.client
-            .waitForTextValue('[data-test-id="bookmarkText"]', this.pageNoTitle)
-        })
-        describe('and then removed', function () {
-          before(function * () {
-            yield this.app.client
-              .click(navigatorNotBookmarked)
-              .waitForExist(removeButton)
-              .click(removeButton)
-          })
-          it('removes the bookmark from the toolbar', function * () {
-            yield this.app.client
-              .waitForExist('[data-test-id="bookmarkText"]', Brave.defaultTimeout, true)
-          })
-        })
-      })
     })
+
     describe('bookmark pdf', function () {
       Brave.beforeAll(this)
 

test/bookmark-components/bookmarksToolbarTest.js

@@ -177,7 +177,7 @@ describe('bookmarksToolbar', function () {
         .waitForVisible(navigatorNotBookmarked)
         .click(navigatorNotBookmarked)
         .waitForVisible(doneButton)
-        .waitForBookmarkDetail(pageWithFavicon, '')
+        .waitForBookmarkDetail(pageWithFavicon, pageWithFavicon.replace(/http:\/\//, ''))
         .waitForEnabled(doneButton)
         .click(doneButton)
 

test/tab-components/frameTest.js

@@ -60,14 +60,20 @@ describe('frame tests', function () {
           .ipcSend('shortcut-set-active-frame-by-index', 0)
           .windowByUrl(Brave.browserWindowUrl)
           .cloneTabByIndex(0)
-          .waitForTabCount(3)
       })
 
       it('inserts after the tab to clone', function * () {
         this.tab1 = '.tabArea:nth-child(1) [data-test-id="tab"][data-frame-key="1"]'
         this.tab2 = '.tabArea:nth-child(2) [data-test-id="tab"][data-frame-key="3"]'
         this.tab3 = '.tabArea:nth-child(3) [data-test-id="tab"][data-frame-key="2"]'
+        const tabUrl = this.clickWithTargetPage
         yield this.app.client
+          .waitForTabCount(3)
+          .waitUntil(function () {
+            return this.tabByIndex(2).getUrl().then((url) => {
+              return url === tabUrl
+            })
+          })
           .windowByUrl(Brave.browserWindowUrl)
           .waitForExist(this.tab1)
           .waitForExist(this.tab2)