Closed Bug 1444760 Opened 7 years ago Closed 7 years ago

Combine loadURIWithFlags with loadURI

Categories

(Firefox :: Tabbed Browser, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(2 files)

Right now, two forms of loadURIWithFlags are supported: loadURIWithFlags(aURI, aFlags, aReferrerURI, aCharset, aPostData); and loadURIWithFlags(aURI, aParams); where aParams is an object containing aFlags, aReferrerURI, aCharset, aPostData. This bug is about supporting only the second form.
Depends on: 1442058
Blocks: 1442058
No longer depends on: 1442058
I'm thinking it would be possible even to combine loadURI and loadURIWithFlags into one single method since loadURI is basically loadURIWithFlags with a reduced number of parameters. We could do that really easily with ES6 default parameters.
Summary: Support only one form of loadURIWithFlags calls → Combine loadURIWithFlags with loadURI
Blocks: 1441935
No longer blocks: 1442058
Comment on attachment 8957960 [details] Bug 1444760 - Support only one form of loadURIWithFlags calls. https://reviewboard.mozilla.org/r/226876/#review232762 ::: browser/base/content/browser.js:5367 (Diff revision 5) > if (aURI) { > let loadflags = isExternal ? > Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL : > Ci.nsIWebNavigation.LOAD_FLAGS_NONE; > gBrowser.loadURIWithFlags(aURI.spec, { > - aTriggeringPrincipal, > + aTriggeringPrincipal, triggeringPrincipal: aTriggeringPrincipal, ::: browser/base/content/tabbrowser.js (Diff revision 5) > aIID.equals(Ci.nsISupports)) > return this; > throw Cr.NS_NOINTERFACE; > } > } > - please drop this change
Attachment #8957960 - Flags: review?(dao+bmo) → review+
Comment on attachment 8957963 [details] Bug 1444760 - Combine loadURIWithFlags and loadURI methods. https://reviewboard.mozilla.org/r/226878/#review234146 Nice cleanup, thanks!
Attachment #8957963 - Flags: review?(dao+bmo) → review+
Blocks: 1387013
Status: NEW → ASSIGNED
(In reply to Dão Gottwald [::dao] from comment #16) > ::: browser/base/content/tabbrowser.js > (Diff revision 5) > > aIID.equals(Ci.nsISupports)) > > return this; > > throw Cr.NS_NOINTERFACE; > > } > > } > > - > > please drop this change Looks like there was 2 empty lines at the end of the file before the change, and Atom made it so there's only one empty line left at the end of the file. Seems like a reasonable change to me, but I'm happy to fight Atom with adding back the second empty line if needed :)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 73f75121ac28326aa83e5dfc2637dede2558c47d -d 6426e089e5c5: rebasing 452734:73f75121ac28 "Bug 1444760 - Support only one form of loadURIWithFlags calls. r=dao" merging browser/base/content/browser.js merging browser/base/content/tabbrowser.js merging browser/base/content/tabbrowser.xml merging toolkit/content/widgets/browser.xml merging toolkit/mozapps/extensions/content/extensions.js rebasing 452735:aaee500f8c9c "Bug 1444760 - Combine loadURIWithFlags and loadURI methods. r=dao" (tip) merging browser/base/content/browser.js merging browser/base/content/tabbrowser.js merging browser/base/content/tabbrowser.xml merging browser/components/extensions/ext-devtools-panels.js merging browser/components/extensions/ext-devtools.js merging browser/components/search/test/browser_aboutSearchReset.js merging browser/components/sessionstore/SessionStore.jsm merging toolkit/content/widgets/browser.xml merging toolkit/mozapps/extensions/content/extensions.js warning: conflicts while merging browser/base/content/tabbrowser.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4c52361ab381 Support only one form of loadURIWithFlags calls. r=dao https://hg.mozilla.org/integration/mozilla-inbound/rev/c1e0ecafa703 Combine loadURIWithFlags and loadURI methods. r=dao
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/08e77f90bdb4 Followup: Fix broken rebase in tabbrowser.js. r=me
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/00da6f45a098 Followup: Fix 'params is undefined' exception on a CLOSED TREE. r=me
Comment on attachment 8957960 [details] Bug 1444760 - Support only one form of loadURIWithFlags calls. https://reviewboard.mozilla.org/r/226876/#review236422 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: mobile/android/modules/geckoview/GeckoViewNavigation.jsm:57 (Diff revision 9) > break; > case "GeckoView:GoForward": > this.browser.goForward(); > break; > case "GeckoView:LoadUri": > const { uri, referrer, baseUri, flags } = aData; Error: 'baseuri' is assigned a value but never used. [eslint: no-unused-vars]
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4956db1e9bdd Support only one form of loadURIWithFlags calls. r=dao https://hg.mozilla.org/integration/autoland/rev/71f61091a716 Combine loadURIWithFlags and loadURI methods. r=dao
Comment on attachment 8957963 [details] Bug 1444760 - Combine loadURIWithFlags and loadURI methods. https://reviewboard.mozilla.org/r/226878/#review236424 Code analysis found 2 defects in this patch: - 2 defects found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/base/content/browser.js:1136 (Diff revision 10) > if (mustChangeProcess) { > Cu.reportError(e); > gBrowser.updateBrowserRemotenessByURL(browser, uri); > > - if (params.userContextId) { > - browser.webNavigation.setOriginAttributesBeforeLoading({ userContextId: params.userContextId }); > + if (userContextId) { > + browser.webNavigation.setOriginAttributesBeforeLoading({ userContextId: userContextId }); Error: Expected property shorthand. [eslint: object-shorthand] ::: mobile/android/modules/geckoview/GeckoViewNavigation.jsm:57 (Diff revision 10) > break; > case "GeckoView:GoForward": > this.browser.goForward(); > break; > case "GeckoView:LoadUri": > const { uri, referrer, baseUri, flags } = aData; Error: 'baseuri' is assigned a value but never used. [eslint: no-unused-vars]
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/cedad7dd5d15 Support only one form of loadURIWithFlags calls. r=dao https://hg.mozilla.org/integration/autoland/rev/14a3a7e06735 Combine loadURIWithFlags and loadURI methods. r=dao
Flags: needinfo?(ntim.bugs)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Blocks: 1442058
Type: defect → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: