Bug 1241085 Parts that affect SeaMonkey: Part 2: Rip out userTypedClear and replace it with more self-documenting stuff Part 3: Actually fix about:privatebrowsing clearing the URL bar Part 4: Fix issues with about:newtab and other initial pages whose URIs now persist after session restore,

This one seems to work. Not sure about the unifiedcomplete in tabbrowser.xml. nssessionstore.js still has a usertypedclear reference but I am unsure what to do with it. My guess is it's set in the same file during sessionstore and identical to usertypedvale. Ripping the favicon changes out would be possible but I would rather not do it. Took a few hours already to get to the point where it works and it might aid when porting other changes. I didn't look at the tests yet but usertypedclear seems not to be used in them.

Afaics, in theory 2.45(a2) should not be affected by Bug 1241085 - but nonetheless, the back and forward buttons are not working most of the time in the same way like described here....

I am having zero problems with them under Windows. Linux, osx or Windows? If Linux which gtk? 3.20 currently seems to be problematic but only read about scrollbar problems. FRG

Comment on attachment 8754787 [details] [diff] [review] 1272401-usertypedclear.patch I think you misunderstood me. When I said to "Port Firefox Bug 1241085 Parts". I meant only the relevant parts that affect SeaMonkey. > + <field name="_unifiedComplete" readonly="true"> > + Components.classes["@mozilla.org/autocomplete/search;1?name=unifiedcomplete"] > + .getService(Components.interfaces.mozIPlacesAutoComplete); > + </field> Some other bug? > + // cache flags for correct status UI update after tab switching > + mTotalProgress: 0, > + > + // count of open requests (should always be 0 or 1) > + mRequestCount: 0, > + > + _callProgressListeners: function () { > + Array.unshift(arguments, this.mBrowser); > + return this.mTabBrowser._callProgressListeners.apply(this.mTabBrowser, arguments); > + }, > + > + _shouldShowProgress: function (aRequest) { > + if (this.mBlank) > + return false; > + > + // Don't show progress indicators in tabs for about: URIs > + // pointing to local resources. > + if ((aRequest instanceof Components.interfaces.nsIChannel) && > + aRequest.originalURI.schemeIs("about") && > + (aRequest.URI.schemeIs("jar") || aRequest.URI.schemeIs("file"))) > + return false; > + > + return true; > + }, Some other bug? > - else if (this.mBrowser.contentDocument instanceof ImageDocument && > - this.mTabBrowser.mPrefs.getBoolPref("browser.chrome.site_icons")) { > - var req = this.mBrowser.contentDocument.imageRequest; > - if (req && !(req.imageStatus & Components.interfaces.imgIRequest.STATUS_ERROR)) { > - try { > - var canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas"); > - var tabImg = document.getAnonymousElementByAttribute(this.mTab, "anonid", "tab-icon"); > - var w = tabImg.boxObject.width; > - var h = tabImg.boxObject.height; > - canvas.width = w; > - canvas.height = h; > - var ctx = canvas.getContext('2d'); > - ctx.drawImage(this.mBrowser.contentDocument.body.firstChild, 0, 0, w, h); > - this.mTab.setAttribute("image", canvas.toDataURL()); > - } catch (e) { // non-canvas build, fall back to the old method > - var sz = this.mTabBrowser.mPrefs.getIntPref("browser.chrome.image_icons.max_size"); > - if (req.image.width <= sz && req.image.height <= sz) > - this.mTab.setAttribute("image", this.mBrowser.currentURI.spec); > - } > - } > - } > - else if (this.mTabBrowser.shouldLoadFavIcon(location)) > - this.mTabBrowser.loadFavIcon(location, "image", this.mTab, > - this.mBrowser.contentPrincipal); > - else > - this.mTab.removeAttribute("image"); Don't remove existing functionality! > + if (oldBlank) { > + this._callProgressListeners("onUpdateCurrentBrowser", > + [aStateFlags, aStatus, "", 0], > + true, false); > + } else { > + this._callProgressListeners("onStateChange", > + [aWebProgress, aRequest, aStateFlags, aStatus], > + true, false); > + } > + > + this._callProgressListeners("onStateChange", > + [aWebProgress, aRequest, aStateFlags, aStatus], > + false); > + > + if (aStateFlags & (nsIWebProgressListener.STATE_START | > + nsIWebProgressListener.STATE_STOP)) { > + // reset cached temporary values at beginning and end > + this.mMessage = ""; > + this.mTotalProgress = 0; > + } Some other bug? > - var browser = aTab.linkedBrowser; > - if (!aURI) > - browser.mIconURL = ""; > - else { > - if (!(aURI instanceof Components.interfaces.nsIURI)) > - aURI = this.mIOService.newURI(aURI, null, null); > - browser.mIconURL = aURI.spec; > + let browser = this.getBrowserForTab(aTab); > + browser.mIconURL = aURI instanceof Components.interfaces.nsIURI ? aURI.spec : aURI; > + > + if (aURI && this.mFaviconService) { > + if (!(aURI instanceof Components.interfaces.nsIURI)) { > + aURI = makeURI(aURI); > + } > + // We do not serialize the principal from within nsSessionStore.js, > + // hence if aLoadingPrincipal is null we default to the > + // systemPrincipal which will allow the favicon to load. > + let loadingPrincipal = aLoadingPrincipal > + ? aLoadingPrincipal > + : Services.scriptSecurityManager.getSystemPrincipal(); > + let loadType = PrivateBrowsingUtils.isWindowPrivate(window) > + ? this.mFaviconService.FAVICON_LOAD_PRIVATE > + : this.mFaviconService.FAVICON_LOAD_NON_PRIVATE; > + > + this.mFaviconService.setAndFetchFaviconForPage( > + browser.currentURI, aURI, false, loadType, null, loadingPrincipal); > } > - if (aURI && this.mFaviconService) { > - var faviconFlags = this.usePrivateBrowsing ? > - this.mFaviconService.FAVICON_LOAD_PRIVATE : > - this.mFaviconService.FAVICON_LOAD_NON_PRIVATE; > - var loadingPrincipal = aLoadingPrincipal || > - this.mSecMan.getSystemPrincipal(); > - > - this.mFaviconService > - .setAndFetchFaviconForPage(browser.currentURI, > - aURI, false, > - faviconFlags, null, > - loadingPrincipal); > + let sizedIconUrl = browser.mIconURL || ""; > + if (sizedIconUrl != aTab.getAttribute("image")) { > + if (sizedIconUrl) > + aTab.setAttribute("image", sizedIconUrl); > + else > + aTab.removeAttribute("image"); > + this._tabAttrModified(aTab, ["image"]); > } > - if (!aTab.hasAttribute("busy") && browser.mIconURL) > - aTab.setAttribute("image", browser.mIconURL); > - else > - aTab.removeAttribute("image"); > - this._tabAttrModified(aTab); > - > - this._callProgressListeners(browser, "onLinkIconAvailable", > - [browser.mIconURL]); > + this._callProgressListeners(browser, "onLinkIconAvailable", [browser.mIconURL]); Some other bug? > + <method name="useDefaultIcon"> > + <parameter name="aTab"/> > + <body> > + <![CDATA[ > + var browser = this.getBrowserForTab(aTab); > + var documentURI = browser.documentURI; > + var icon = null; > + > + if (browser.imageDocument) { > + if (Services.prefs.getBoolPref("browser.chrome.site_icons")) { > + let sz = Services.prefs.getIntPref("browser.chrome.image_icons.max_size"); > + if (browser.imageDocument.width <= sz && > + browser.imageDocument.height <= sz) { > + icon = browser.currentURI; > + } > + } > + } > + > + // Use documentURIObject in the check for shouldLoadFavIcon so that we > + // do the right thing with about:-style error pages. Bug 453442 > + if (!icon && this.shouldLoadFavIcon(documentURI)) { > + let url = documentURI.prePath + "/favicon.ico"; > + if (!this.isFailedIcon(url)) > + icon = url; > + } > + this.setIcon(aTab, icon, browser.contentPrincipal); > + ]]> > + </body> > + </method> > + > + <method name="isFailedIcon"> > + <parameter name="aURI"/> > + <body> > + <![CDATA[ > + if (this.mFaviconService) { > + if (!(aURI instanceof Ci.nsIURI)) > + aURI = makeURI(aURI); > + return this.mFaviconService.isFailedFavicon(aURI); > + } > + return null; > + ]]> > + </body> > + </method> > + Some other bug? > + <parameter name="aChanged"/> > <body><![CDATA[ > - // This event should be dispatched when any of these attributes change: > - // label, crop, busy, image, selected > - aTab.dispatchEvent(new Event("TabAttrModified", > - { bubbles: true, cancelable: false })); > + if (aTab.closing) > + return; > + > + let event = new CustomEvent("TabAttrModified", { > + bubbles: true, > + cancelable: false, > + detail: { > + changed: aChanged, > + } > + }); > + aTab.dispatchEvent(event); > ]]></body> > </method> > + <method name="setTabTitleLoading"> > + <parameter name="aTab"/> > + <body> > + <![CDATA[ > + aTab.label = this.mStringBundle.getString("tabs.loading"); > + aTab.crop = "end"; > + this._tabAttrModified(aTab, ["label", "crop"]); > + ]]> > + </body> > + </method> Some other bug? > - var buttonPressed = promptService.confirmEx(window, > - bundle.getString('tabs.closeWarningTitle'), > + var buttonPressed = promptService.confirmEx(window, > + bundle.getString('tabs.closeWarningTitle'), > - if (this.tabs[i] == aTab) > + if (this.tabs[i] == aTab) Whitespace clean up goes into a separate patch. Can be in this bug but a separate patch please. > +this.__defineGetter__("BROWSER_NEW_TAB_URL", () => { > + if (PrivateBrowsingUtils.isWindowPrivate(window) && > + !PrivateBrowsingUtils.permanentPrivateBrowsing) { > + return "about:privatebrowsing"; > + } > + return ""; > +}); > + Some other bug? Note we never had the "BROWSER_NEW_TAB_URL" API. If you want to port this you should port all of it or none of it and not just a random chunk that does nothing without the rest. And in a separate bug! > +function isBlankPageURL(aURL) { > + return aURL == "about:blank" || aURL == BROWSER_NEW_TAB_URL; > +} Ditto Now can we try again? Thanks.

Found two errors in the code 1 new and one old. Stripped out some parts like unifiedcomplete from the patch which I verified are not needed. For the rest I would need to dig so deep into the code that it was just easier to keep the parts as stated on irc.

Version due to Flags

Changing to blocker (unfortunately I don't see any seamonkey-release-blocker flags...), as we cannot release a browser with the address bar lying about the current URL.

Work fine for me with with English SeaMonkey 2.45 (unofficial by frg) (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 Build 20160628141709 (Default Classic Theme) on German WIN7 64bit: "Bug 1287078 - Location Bar contents not updated when opening new page in the same TAB by link, History or bookmark" "Bug 1286282 - As I navigate the Web, the location bar isn't updated any more" "Bug 1287255 - Drag and Dop Bookmark from Location bar to Bookmarks Toolbar or sidebar Bookmark Panel fails" It seems that version contains a patch?

Work fine for me with English SeaMonkey 2.45a1 (unofficial by frg) (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 Build 20160716124751 (Default Classic Theme) on German WIN7 64bit

User agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a1 Build identifier: 20160719003002 http://hg.mozilla.org/mozilla-central/rev/feaaf1af1065257b9178faca8b67eed9657b4a17 http://hg.mozilla.org/comm-central/rev/bbdd29586adf The current Trunk nightly still doesn't update its URL bar when browsing remote pages. (For local "file:///something" pages it usually does.)

Comment on attachment 8762112 [details] [diff] [review] 1272401-usertypedclear-V2.patch >+++ b/suite/browser/tabbrowser.xml >+ if (aWebProgress.isTopLevel) { >+ // Need to use originalLocation rather than location because things >+ // like about:home and about:privatebrowsing arrive with nsIRequest Nit: SM doesn't have about:home (yet) >+ // pointing to their resolved jar: or file: URIs. >+ if (!(originalLocation && gInitialPages.has(originalLocation.spec) && >+ originalLocation != "about:blank" && >+ this.mBrowser.currentURI && this.mBrowser.currentURI.spec == "about:blank")) { >+ // This will trigger clearing the location bar. Don't do it if >+ // we loaded off a blank browser and this is an initial page load >+ // (e.g. about:privatebrowsing, about:newtab, etc.) so we avoid Nit: SM doesn't have about:newtab >+ // If the browser is loading it must not be crashed anymore Nit: Full stop at the end of the comment. >+ // For keyword URIs clear the user typed value since they will be changed into real URIs Nit: Full stop at the end of the comment. >+ onLocationChange: function (aWebProgress, aRequest, aLocation, >+ aFlags) { Nit: "aFlags {" is within the 80 character limit so could be on the line above. >+ if (aWebProgress.isLoadingDocument && !isSameDocument) { >+ this.mBrowser.mIconURL = null; >+ } Existing style seems to be that single line if statements do not have braces. >+ if (!this.mBlank) { >+ this._callProgressListeners("onLocationChange", >+ [aWebProgress, aRequest, aLocation, >+ aFlags]); >+ } Existing style seems to be that single line if statements do not have braces. >+ if (aURI && this.mFaviconService) { >+ if (!(aURI instanceof Components.interfaces.nsIURI)) { >+ aURI = makeURI(aURI); >+ } Existing style seems to be that single line if statements do not have braces. r=me with those addressed / fixed.

User agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a1 Build identifier: 20160720112903 http://hg.mozilla.org/mozilla-central/rev/d224fc999cb6accb208af0a105f14433375e2e77 http://hg.mozilla.org/comm-central/rev/7f04347eceee Some time ago the Location Bar was frozen across links when browsing remote pages but not local "file:" pages. In this build (and possibly a few nightlies before it) it is the opposite: following links on Wikipedia or on my home site on the Web updates the Location Bar, but browsing HTML pages on my own HD (working copies of my home site) doesn't. FWIW, when I browse http://users.skynet.be/antoine.mechelynck/slovarj/ru-fr.abbrev.html and the pages accessible from there (А Б В and part of Г, at the moment: that ru-fr dictionary is a WIP only recently begun) /in loco/ on the Web, the URL bar is updated; but when I browse identical copies of the same pages on my HD, it isn't. OTOH the local pages have fully functional Back/Forward buttons but on the remote pages they are quirky: whether one or the other is greyed-out and whether clicking it does anything seems only vaguely related to which history pages are available (and accessible e.g. at the bottom of the Go menu).

P.S. In case you want to download my dictionary pages and try to see what I see, they are best viewed with the http://users.kynet.be/antoine.mechelynck/slovarj/ru-fr.css style sheet and they invoke three unimportant images in the same directory.

Oops! http://users.skynet.be/antoine.mechelynck/slovarj/ru-fr.css That's what I get when typing too fast.

(In reply to Tony Mechelynck [:tonymec] from comment #16) On this Bugzilla page, OTOH, going from one comment to another by clicking links does _not_ update the URL bar.

Nits fixed. I noticed a small difference in my current file in the setIcon method. I took mine to make sure it is the version I tested with the last month. Review+ from IanN carried forward.

Testing the following tinderbox-build, made one c-c changeset later than the CS mentioned in comment #21: User agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a1 Build identifier: 20160721161703 http://hg.mozilla.org/mozilla-central/rev/2e3390571fdb3a1ff3d2f7f828adf67dbc237bc8 http://hg.mozilla.org/comm-central/rev/a09c05db1261 The Location Bar now follows links on this page and elsewhere on the Web, at least after a refresh. On my local HD (whose pages are not cached and therefore usually don't need a refresh to display the latest version) the Location Bar follows same-page links _only_ after a refresh. (Ctrl+Shift+R made the Location Bar follow links, I didn't try unshifted Ctrl+R). Links to other pages in the same directory updated the Location Bar immediately. After posting this comment I'll check whether the Location Bar still follows Web locations after coming back from a POST command.

(In reply to Tony Mechelynck [:tonymec] from comment #22) > After posting this comment I'll check whether the Location Bar still follows > Web locations after coming back from a POST command. AFAICT it does.

P.S. (to comment #22) Session Manager extension is installed and enabled. I forgot to cycle through Safe Mode.

Comment on attachment 8773444 [details] [diff] [review] 1272401-usertypedclear-V3.patch [Approval Request Comment] Regression caused by (bug #): User impact if declined: broken location bar and unusable program Testing completed (on m-c, etc.): c-b c-a Risk to taking this patch (and alternatives if risky): none. It needs to be done or 245 and 2.46 could not be shipped. String changes made by this patch: none. I also forgot to fix one nit (curly braces not removed). I would like to put this in too: + if (this.mTab.selected && gURLBar && !inLoadURI) { + URLBarSetURI(); + }

Comment on attachment 8773444 [details] [diff] [review] 1272401-usertypedclear-V3.patch r/a=me for the additional change and landing on c-a/c-b

Nitfix for comm-central r+ by IanN in previous comment.

Patch for Beta and Aurora including the nitfix approved by IanN see previous comments.

https://hg.mozilla.org/comm-central/rev/3acada2d4bff https://hg.mozilla.org/releases/comm-aurora/rev/4fcea516d2a9 https://hg.mozilla.org/releases/comm-beta/rev/3904a4a0fdf5 Resolved for now. If problems pop up will either be reopened or dealt with in a followup patch.