The default bug view has changed. See this FAQ.

sync last screen and last uri preference

RESOLVED FIXED

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

unspecified
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

6 years ago
Priority: -- → P1
(Assignee)

Comment 1

6 years ago
Created attachment 568994 [details] [diff] [review]
work in progress.

pretty close to what we want.  I want to see if we can also store the offset
Attachment #568994 - Flags: review-
(In reply to Doug Turner (:dougt) from comment #1)
> Created attachment 568994 [details] [diff] [review] [diff] [details] [review]
> work in progress.
> 
> pretty close to what we want.  I want to see if we can also store the offset

Sounds like something we want in session restore anyway
(Assignee)

Comment 3

6 years ago
if so, maybe we go with this as-is.  it is a ton better now.

the basic idea is that we can't keep the last uri in prefs because we need it before gecko starts up.
(Assignee)

Updated

6 years ago
Attachment #568994 - Flags: review- → review?
Comment on attachment 568994 [details] [diff] [review]
work in progress.


>diff --git a/embedding/android/GeckoApp.java b/embedding/android/GeckoApp.java

>+                    // this is already true
>+                    // surfaceView.showStartupBitmap();

Remove these

>+                    final String awesomeURI = uri; 
>+                    mMainHandler.post(new Runnable() {
>+                      public void run() {
>+                        mBrowserToolbar.setTitle(awesomeURI);
>+                      }
>+                    });

Does this pass the blassey-test for mMainHandler vs. mHandler ?

Also, can't we try to use "last-title" pref here?

>+    private void rememberLastScreen(boolean sync) {

>+        if (!tab.getHistory().empty()) {
>+
>+            SharedPreferences prefs = getSharedPreferences ("GeckoApp", 0);

nits: no blank line needed at top of block and drop the space from | getSharedPreferences ( | (you do it every time you use getSharedPreferences)

>+            surfaceView.saveLast(sync);
>+        }
>+
>+    }

no blank line needed here

>+    private boolean restoreLastScreen() {

>+        return true;
>+        //        surfaceView.saveLast(sync);

remove

>     private void quit() {

>+        System.exit(0);
>         GeckoAppShell.nativeQuit();
>         finish();

Brad removed System.exit(0) and added the 2 new lines in this patch:
http://hg.mozilla.org/projects/birch/rev/653aa43e9643

Do we need it?

>diff --git a/embedding/android/GeckoSurfaceView.java b/embedding/android/GeckoSurfaceView.java

>     public void surfaceCreated(SurfaceHolder holder) {

>+        Log.i(LOG_FILE_NAME, "!! surface created");
>+
>+        // Delay sending this event if we are painting the
>+        // load screen.  The native access paint path will
>+        // paint directly to the screen and we will see a
>+        // black screen while content is initally being
>+        // drawn.
>+        if (!mShowingLoadScreen) {
>+            GeckoEvent e = new GeckoEvent(GeckoEvent.SURFACE_CREATED);
>+            GeckoAppShell.sendEventToGecko(e);
>+        }

You might want to move the Log into the if block, so we know if this is _really_ called

>diff --git a/mobile/components/BrowserCLH.js b/mobile/components/BrowserCLH.js

> BrowserCLH.prototype = {
>   handle: function fs_handle(aCmdLine) {
>-    dump("fs_handle");
> 
>-    let urlParam = aCmdLine.handleFlagWithParam("remote", false);

Don't leave the leading blank line

>+    let urlParam = "about:";

about:home? or do you want to wait?

>+    try {
>+        urlParam = aCmdLine.handleFlagWithParam("remote", false);

2 spaces

>+    } catch (e) { dump(""+e); }

nit: dump("" + e);

r+ with nits and some thought for the System.exit(0)

the mMainHandler things is up to you :)
Attachment #568994 - Flags: review? → review+
(Assignee)

Comment 5

6 years ago
> Does this pass the blassey-test for mMainHandler vs. mHandler ?

There are like 2 dozen similar patterns here for setting the ui widgets.  we can have a follow up.

> Also, can't we try to use "last-title" pref here?

Not sure what you mean.  We can set the title instead, either way.
(Assignee)

Comment 6

6 years ago
http://hg.mozilla.org/projects/birch/rev/45c91268cfee
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.