Last Comment Bug 663040 - Implement webNavigation for remote browsers
: Implement webNavigation for remote browsers
Status: RESOLVED FIXED
[e10s]
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: mozilla8
Assigned To: :Felipe Gomes (needinfo me!)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-08 23:25 PDT by :Felipe Gomes (needinfo me!)
Modified: 2011-07-08 13:17 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.54 KB, patch)
2011-06-08 23:25 PDT, :Felipe Gomes (needinfo me!)
no flags Details | Diff | Splinter Review
Patch (8.04 KB, patch)
2011-06-17 19:03 PDT, :Felipe Gomes (needinfo me!)
dtownsend: review+
mark.finkle: review+
Details | Diff | Splinter Review
Fix (1.39 KB, patch)
2011-07-07 13:31 PDT, :Felipe Gomes (needinfo me!)
mark.finkle: review+
Details | Diff | Splinter Review

Description :Felipe Gomes (needinfo me!) 2011-06-08 23:25:57 PDT
Created attachment 538186 [details] [diff] [review]
Patch v1

Simple patch v1 with implementation copied straight from mobile. Content-side support for it will come from bug 662008.

Still pondering if I should remove it from mobile or let it be overridden
Comment 1 :Felipe Gomes (needinfo me!) 2011-06-17 19:03:18 PDT
Created attachment 540196 [details] [diff] [review]
Patch

This just moves the remoteWebNavigation from mobile's browser.xml#remote-browser to toolkit's browser.xml

The getter in mobile is not removed so that it can skip the isRemoteBrowser check, as fennec has two different bindings for remote and non-remote browser.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-06-23 13:13:39 PDT
Comment on attachment 540196 [details] [diff] [review]
Patch

>diff --git a/toolkit/content/widgets/browser.xml b/toolkit/content/widgets/browser.xml

>+      <property name="isRemoteBrowser"
>+                onget="return (this.getAttribute('remote') == 'true');"
>+                readonly="true"/>
>+

Just wanted to note my disapproval of this kind of code. I think the dual binding approach serves us better.
Comment 3 Marco Bonardo [::mak] 2011-07-01 07:48:19 PDT
I backed out everything from central since Android and Maemo were unhappy about the push these changes were part of.
Comment 4 :Felipe Gomes (needinfo me!) 2011-07-07 13:31:04 PDT
Created attachment 544599 [details] [diff] [review]
Fix

Mark, this is why the back/forward, thumbnails and pinch-to-zoom broke. WebNav was renamed to _remoteWebNavigation with this patch, and the getter was updated, but this code was accessing the internal field instead of the public getter so it wasn't updating the correct object.
Comment 5 :Felipe Gomes (needinfo me!) 2011-07-08 13:17:49 PDT
http://hg.mozilla.org/mozilla-central/rev/4fec0adad951

Note You need to log in before you can comment on or make changes to this bug.