Closed
Bug 1445080
Opened 7 years ago
Closed 7 years ago
remote-browser depends on consumer to setup correctly
Categories
(Toolkit :: XUL Widgets, defect)
Tracking
()
VERIFIED
FIXED
mozilla61
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Gijs
:
review+
mconley
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
In a followup to bug 1443749, I discovered that a remote browser (in sidebar) did not have currentURI set correctly after a document was loaded. This likely also affects browser and page actions, though we don't hit this specific issue as we are not touching currentURI.
By adding: "browser.webProgress;" after inserting the browser into the document, RemoteWebProgress is created and currentURI is set correctly. We shouldn't depend on the consumer of the remote browser doing this.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
This is going to need uplift to 60 in order to fix issues with sidebar extensions, such as Notes and Tab Split.
status-firefox59:
--- → affected
status-firefox60:
--- → affected
status-firefox61:
--- → affected
tracking-firefox60:
--- → ?
Assignee | ||
Updated•7 years ago
|
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8958277 [details]
Bug 1445080 - fix handling of remote web progress for non-tab browsers,
https://reviewboard.mozilla.org/r/227210/#review233138
Good news, even in the bright light of morning, this looks sensible to me, assuming Mike agrees. :-)
::: toolkit/content/widgets/remote-browser.xml:53
(Diff revision 1)
> readonly="true"/>
>
> <field name="_remoteWebProgress">null</field>
>
> <property name="webProgress" readonly="true">
> <getter>
Nit: can you fix the tabs while you're here?
Attachment #8958277 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8958277 [details]
Bug 1445080 - fix handling of remote web progress for non-tab browsers,
https://reviewboard.mozilla.org/r/227210/#review233282
Yeah, this looks reasonable. Thanks, mixedpuppy!
::: commit-message-d6957:1
(Diff revision 2)
> +Bug 1445080 fix handling of remote web progress for non-tab browsers, r?Gijs,mconley
Nit: I think normally a - or a : goes between the bub number and the commit message. Maybe reformat as:
Bug 1445080 - Fix handling of remote web progress for non-tab browsers. r?Gijs,mconley
Attachment #8958277 -
Flags: review?(mconley) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4c204ccf3a7c
fix handling of remote web progress for non-tab browsers, r=Gijs,mconley
Comment 11•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8958277 [details]
Bug 1445080 - fix handling of remote web progress for non-tab browsers,
Approval Request Comment
[Feature/Bug causing the regression]: remote-browser
[User impact if declined]: non-tab browser used in sidebar reloads too often
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: Verifying fixes that resulted in these bugs would be good even though there is a test. QA for notes and tab-split has several github issues with STR
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: Risk is moderate.
[Why is the change risky/not risky?]: The change is simple, however it affects all remote browser elements
[String changes made/needed]: none
Attachment #8958277 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
Updated•7 years ago
|
Flags: qe-verify+
Comment 13•7 years ago
|
||
This seemed to fix the problems that Notes had, I am gonna go through the bugs we have in Github and SoftVision will do another run at this. Thank you!
Comment 14•7 years ago
|
||
Comment on attachment 8958277 [details]
Bug 1445080 - fix handling of remote web progress for non-tab browsers,
fix issues with sidebar extensions, beta60+
Attachment #8958277 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
Flags: in-testsuite+
Comment 16•7 years ago
|
||
We've retested the Firefox Notes and Tab Split issues filed from the corresponding GitHub repositories and the issues are no longer reproducible on latest Nightly build 61.0a1 (Build ID: 20180321220044) and latest Beta 60.0b5 (Build ID: 20180319175655), on Windows 10 x64, Mac 10.13.3, Ubuntu 14.04 x64 and Arch Linux 4.12.
From the 2 webextensions side, this change fixed the problems. However I'm not sure how can we test if the currentURI is set correctly, and confirm that everything fixed in this issue is as it should be.
Shane, can you please provide a minimal test case and some information regarding how we can QA this?
Flags: needinfo?(mixedpuppy)
Assignee | ||
Comment 17•7 years ago
|
||
An automated test was added for currentURI. The only manual verification necessary is that the extensions were fixed.
Flags: needinfo?(mixedpuppy)
Comment 18•7 years ago
|
||
All right, we are good then. Thank you!
Comment 19•7 years ago
|
||
Based on previous comments, this bug was verified on Nightly 60.0a1 and Beta 60.0b5. Removing the qe-verify+ flag.
Flags: qe-verify+
Updated•6 years ago
|
Assignee: nobody → mixedpuppy
You need to log in
before you can comment on or make changes to this bug.
Description
•